Re: [HACKERS] about EDITOR_LINENUMBER_SWITCH
Here's a patch to fix what has been discussed: * Change EDITOR_LINENUMBER_SWITCH to environment variable. * I also changed switch to arg because switch is a bit of a sloppy term. * So the environment variable is called PSQL_EDITOR_LINENUMBER_ARG. * Set + as hardcoded default value on Unix (since vi is the hardcoded default editor), so many users won't have to configure this at all. * I moved the documentation around a bit to centralize the editor configuration under environment variables, rather than repeating bits of it under every backslash command that invoked an editor. *** i/doc/src/sgml/ref/psql-ref.sgml --- w/doc/src/sgml/ref/psql-ref.sgml *** *** 1440,1464 testdb=gt; literal\r/ to cancel. /para - tip para ! applicationpsql/application checks the environment ! variables envarPSQL_EDITOR/envar, envarEDITOR/envar, and ! envarVISUAL/envar (in that order) for an editor to use. If ! all of them are unset, filenamevi/filename is used on Unix ! systems, filenamenotepad.exe/filename on Windows systems. /para - /tip para ! If a line number is specified, applicationpsql/application will ! position the cursor on the specified line of the file or query buffer. ! This feature requires the varnameEDITOR_LINENUMBER_SWITCH/varname ! variable to be set, so that applicationpsql/application knows how ! to specify the line number to the editor. Note that if a single ! all-digits argument is given, applicationpsql/application assumes ! it is a line number not a file name. /para /listitem /varlistentry --- 1440,1460 literal\r/ to cancel. /para para ! If a line number is specified, applicationpsql/application will ! position the cursor on the specified line of the file or query buffer. ! Note that if a single all-digits argument is given, ! applicationpsql/application assumes it is a line number ! not a file name. /para + tip para ! See under xref linkend=app-psql-environment ! endterm=app-psql-environment-title for how to configure and ! customize your editor. /para + /tip /listitem /varlistentry *** *** 1514,1526 Tue Oct 26 21:40:57 CEST 1999 para If a line number is specified, applicationpsql/application will ! position the cursor on the specified line of the function body ! (note that the function body typically does not begin on the ! first line of the file). ! This feature requires the varnameEDITOR_LINENUMBER_SWITCH/varname ! variable to be set, so that applicationpsql/application knows how ! to specify the line number to the editor. /para /listitem /varlistentry --- 1510,1527 para If a line number is specified, applicationpsql/application will ! position the cursor on the specified line of the function body. ! (Note that the function body typically does not begin on the first ! line of the file.) ! /para ! ! tip ! para ! See under xref linkend=app-psql-environment ! endterm=app-psql-environment-title for how to configure and ! customize your editor. /para + /tip /listitem /varlistentry *** *** 2599,2625 bar /varlistentry varlistentry - termvarnameEDITOR_LINENUMBER_SWITCH/varname/term - listitem - para - When command\edit/command or command\ef/command is used with a - line number argument, this variable specifies the command-line switch - used to pass the line number to the user's editor. For editors such - as productnameemacs/ or productnamevi/, you can simply set - this variable to a plus sign. Include a trailing space in the value - of the variable if there needs to be space between the switch name and - the line number. - Examples: - - programlisting - \set EDITOR_LINENUMBER_SWITCH + - \set EDITOR_LINENUMBER_SWITCH '--line ' - /programlisting - /para - /listitem - /varlistentry - - varlistentry termvarnameENCODING/varname/term listitem para --- 2600,2605 *** *** 3167,3174 $endif /refsect1 ! refsect1 ! titleEnvironment/title variablelist --- 3147,3154 /refsect1 ! refsect1 id=app-psql-environment ! title id=app-psql-environment-titleEnvironment/title variablelist *** *** 3218,3225 $endif listitem para !
[HACKERS] include host names in hba error messages
Since we are accepting host names in pg_hba.conf now, I figured it could
be useful to also show the host names in error message, e.g.,
no pg_hba.conf entry for host localhost (127.0.0.1), user x, database
y
Attached is an example patch. The question might be what criterion to
use for when to show the host name. It could be
if (port-remote_hostname_resolv == +1)
that is, we have done the reverse and forward lookup, or
if (port-remote_hostname_resolv = 0)
that is, we have only done the reverse lookup (which is consistent with
log_hostname).
Although this whole thing could be quite weird, because the message that
a host name was rejected because the forward lookup didn't match the IP
address is at DEBUG2, so it's usually never shown. So if we tell
someone that there is 'no pg_hba.conf entry for host foo', even though
there is clearly a line saying foo in the file, it would be confusing.
Ideas?
diff --git i/src/backend/libpq/auth.c w/src/backend/libpq/auth.c
index 7799111..3701672 100644
--- i/src/backend/libpq/auth.c
+++ w/src/backend/libpq/auth.c
@@ -442,33 +442,61 @@ ClientAuthentication(Port *port)
if (am_walsender)
{
#ifdef USE_SSL
- ereport(FATAL,
- (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
- errmsg(no pg_hba.conf entry for replication connection from host \%s\, user \%s\, %s,
- hostinfo, port-user_name,
- port-ssl ? _(SSL on) : _(SSL off;
+ if (port-remote_hostname_resolv == +1)
+ ereport(FATAL,
+(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+ errmsg(no pg_hba.conf entry for replication connection from host \%s\ (%s), user \%s\, %s,
+ port-remote_hostname, hostinfo, port-user_name,
+ port-ssl ? _(SSL on) : _(SSL off;
+ else
+ ereport(FATAL,
+(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+ errmsg(no pg_hba.conf entry for replication connection from host \%s\, user \%s\, %s,
+ hostinfo, port-user_name,
+ port-ssl ? _(SSL on) : _(SSL off;
#else
- ereport(FATAL,
- (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
- errmsg(no pg_hba.conf entry for replication connection from host \%s\, user \%s\,
- hostinfo, port-user_name)));
+ if (port-remote_hostname_resolv == +1)
+ ereport(FATAL,
+(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+ errmsg(no pg_hba.conf entry for replication connection from host \%s\ (%s), user \%s\,
+ port-remote_hostname, hostinfo, port-user_name)));
+ else
+ ereport(FATAL,
+(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+ errmsg(no pg_hba.conf entry for replication connection from host \%s\, user \%s\,
+ hostinfo, port-user_name)));
#endif
}
else
{
#ifdef USE_SSL
- ereport(FATAL,
- (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
- errmsg(no pg_hba.conf entry for host \%s\, user \%s\, database \%s\, %s,
- hostinfo, port-user_name,
- port-database_name,
- port-ssl ? _(SSL on) : _(SSL off;
+ if (port-remote_hostname_resolv == +1)
+ ereport(FATAL,
+(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+ errmsg(no pg_hba.conf entry for host \%s\ (%s), user \%s\, database \%s\, %s,
+ port-remote_hostname, hostinfo, port-user_name,
+ port-database_name,
+ port-ssl ? _(SSL on) : _(SSL off;
+ else
+ ereport(FATAL,
+(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+ errmsg(no pg_hba.conf entry for host \%s\, user \%s\, database \%s\, %s,
+ hostinfo, port-user_name,
+ port-database_name,
+ port-ssl ? _(SSL on) : _(SSL off;
#else
- ereport(FATAL,
- (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
- errmsg(no pg_hba.conf entry for host \%s\, user \%s\, database \%s\,
- hostinfo, port-user_name,
- port-database_name)));
+ if (port-remote_hostname_resolv == +1)
+ ereport(FATAL,
+(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+ errmsg(no pg_hba.conf entry for host \%s\ (%s), user \%s\, database \%s\,
+ port-remote_hostname, hostinfo, port-user_name,
+ port-database_name)));
+ else
+ ereport(FATAL,
+(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+ errmsg(no pg_hba.conf entry for host \%s\, user \%s\, database \%s\,
+ hostinfo, port-user_name,
+ port-database_name)));
#endif
}
break;
--
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: [COMMITTERS] pgsql: Cascading replication feature for streaming log-based replicatio
On Tue, Jul 19, 2011 at 11:44 AM, Simon Riggs si...@2ndquadrant.com wrote:
Cascading replication feature for streaming log-based replication.
Standby servers can now have WALSender processes, which can work with
either WALReceiver or archive_commands to pass data. Fully updated
docs, including new conceptual terms of sending server, upstream and
downstream servers. WALSenders terminated when promote to master.
Fujii Masao, review, rework and doc rewrite by Simon Riggs
Thanks a lot for the commit!
You added new GUC category Sending Server(s) into the doc. According to
this change, we need to change also guc.c and postgresql.conf.sample.
Attached patch does that.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***
*** 557,562 const char *const config_group_names[] =
--- 557,564
gettext_noop(Write-Ahead Log / Archiving),
/* REPLICATION */
gettext_noop(Replication),
+ /* REPLICATION_SENDING */
+ gettext_noop(Replication / Sending Server(s)),
/* REPLICATION_MASTER */
gettext_noop(Replication / Master Server),
/* REPLICATION_STANDBY */
***
*** 1918,1924 static struct config_int ConfigureNamesInt[] =
},
{
! {wal_keep_segments, PGC_SIGHUP, REPLICATION_MASTER,
gettext_noop(Sets the number of WAL files held for standby servers.),
NULL
},
--- 1920,1926
},
{
! {wal_keep_segments, PGC_SIGHUP, REPLICATION_SENDING,
gettext_noop(Sets the number of WAL files held for standby servers.),
NULL
},
***
*** 1986,1992 static struct config_int ConfigureNamesInt[] =
{
/* see max_connections */
! {max_wal_senders, PGC_POSTMASTER, REPLICATION_MASTER,
gettext_noop(Sets the maximum number of simultaneously running WAL sender processes.),
NULL
},
--- 1988,1994
{
/* see max_connections */
! {max_wal_senders, PGC_POSTMASTER, REPLICATION_SENDING,
gettext_noop(Sets the maximum number of simultaneously running WAL sender processes.),
NULL
},
***
*** 1996,2002 static struct config_int ConfigureNamesInt[] =
},
{
! {wal_sender_delay, PGC_SIGHUP, REPLICATION_MASTER,
gettext_noop(WAL sender sleep time between WAL replications.),
NULL,
GUC_UNIT_MS
--- 1998,2004
},
{
! {wal_sender_delay, PGC_SIGHUP, REPLICATION_SENDING,
gettext_noop(WAL sender sleep time between WAL replications.),
NULL,
GUC_UNIT_MS
***
*** 2007,2013 static struct config_int ConfigureNamesInt[] =
},
{
! {replication_timeout, PGC_SIGHUP, REPLICATION_MASTER,
gettext_noop(Sets the maximum time to wait for WAL replication.),
NULL,
GUC_UNIT_MS
--- 2009,2015
},
{
! {replication_timeout, PGC_SIGHUP, REPLICATION_SENDING,
gettext_noop(Sets the maximum time to wait for WAL replication.),
NULL,
GUC_UNIT_MS
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***
*** 194,212
# REPLICATION
#--
! # - Master Server -
! # These settings are ignored on a standby server
#max_wal_senders = 0 # max number of walsender processes
# (change requires restart)
#wal_sender_delay = 1s # walsender cycle time, 1-1 milliseconds
#wal_keep_segments = 0 # in logfile segments, 16MB each; 0 disables
- #vacuum_defer_cleanup_age = 0 # number of xacts by which cleanup is delayed
#replication_timeout = 60s # in milliseconds; 0 disables
#synchronous_standby_names = '' # standby servers that provide sync rep
# comma-separated list of application_name
# from standby(s); '*' = all
# - Standby Servers -
--- 194,217
# REPLICATION
#--
! # - Sending Server(s) -
! # These settings have effect on any server that is to send replication data
#max_wal_senders = 0 # max number of walsender processes
# (change requires restart)
#wal_sender_delay = 1s # walsender cycle time, 1-1 milliseconds
#wal_keep_segments = 0 # in logfile segments, 16MB each; 0 disables
#replication_timeout = 60s # in milliseconds; 0 disables
+
+ # - Master Server -
+
+ # These settings are ignored on a standby server
+
#synchronous_standby_names = '' # standby servers that provide sync rep
# comma-separated list of application_name
# from standby(s); '*' = all
+ #vacuum_defer_cleanup_age = 0 # number of xacts by which cleanup is delayed
# - Standby Servers -
*** a/src/include/utils/guc_tables.h
--- b/src/include/utils/guc_tables.h
***
*** 69,74 enum config_group
--- 69,75
WAL_CHECKPOINTS,
WAL_ARCHIVING,
REPLICATION,
+
[HACKERS] range types and ip4r
Just wondering, will the planned range type functionality also be able to absorb the functionality of the ip4r type as a range of the ip4 type (http://pgfoundry.org/projects/ip4r)? Maybe it's trivial, but since the ip types also have a kind of hierarchical structure, I figured I'd point it out in case you hadn't considered it. -- 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: [COMMITTERS] pgsql: Cascading replication feature for streaming log-based replicatio
On Tue, Jul 19, 2011 at 7:31 AM, Fujii Masao masao.fu...@gmail.com wrote: On Tue, Jul 19, 2011 at 11:44 AM, Simon Riggs si...@2ndquadrant.com wrote: Cascading replication feature for streaming log-based replication. Standby servers can now have WALSender processes, which can work with either WALReceiver or archive_commands to pass data. Fully updated docs, including new conceptual terms of sending server, upstream and downstream servers. WALSenders terminated when promote to master. Fujii Masao, review, rework and doc rewrite by Simon Riggs Thanks a lot for the commit! You added new GUC category Sending Server(s) into the doc. According to this change, we need to change also guc.c and postgresql.conf.sample. Attached patch does that. Applied -- Simon Riggs 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] Reduced power consumption in autovacuum launcher process
On 18 July 2011 20:06, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Hmm. Well, it's not too late to rethink the WaitLatch API, if we think that that might be a significant limitation. Right, we can easily change the timeout argument to be in milliseconds instead of microseconds. +1 -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] Exclude core dumps from project - example patch
Hi. I am learn the git. For test my skills, here is patch to exclude core dumps from git tree. Author: pasman pasma...@gmail.com 2011-07-19 10:27:50 Committer: pasman pasma...@gmail.com 2011-07-19 10:27:50 Parent: 6307fff3586294214e3f256035b82bbba9a9054a (Fix typo) Branch: master Follows: REL9_1_BETA2 Precedes: Include core dumps in .gitignore -- .gitignore -- index 1e15ce5..64e3dee 100644 @@ -21,6 +21,8 @@ lcov.info win32ver.rc *.exe lib*dll.def +#Exclude core dumps on Mingw32 +*.stackdump # Local excludes in root directory /GNUmakefile -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cascade replication
On Mon, Jul 11, 2011 at 7:28 AM, Fujii Masao masao.fu...@gmail.com wrote: Attached is the updated version which addresses all the issues raised by Simon. Is there any reason why we disallow cascading unless hot standby is enabled? ISTM we can just alter the postmaster path for walsenders, patch attached. Some people might be happier if a sync standby were not HS enabled, yet able to cascade to other standbys for reading. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services allow_cascading_without_hot_standby.v1.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] [v9.1] sepgsql - userspace access vector cache
On 2011-07-18 22:21, Kohei KaiGai wrote: The Scientific Linux 6 is not suitable, because its libselinux version is a bit older than this patch expects (libselinux-2.0.99 or later). My recommendation is Fedora 15, instead. Installing right now, thanks for the heads up! /etc/selinux/targeted/contexts/sepgsql_contexts: line 33 has invalid object type db_blobs It is not an error, but just a notification to inform users that sepgsql_contexts file contains invalid lines. It is harmless, so we can ignore them. I don't think sepgsql.sgml should mention about this noise, because it purely come from the problem in libselinux and refpolicy; these are external packages from viewpoint of PostgreSQL. This is in contradiction with the current phrase in the documentation that's right after the sepgsql.sql loading: If the installation process completes without error, you can now start the server normally. IMHO if there are warnings that can be ignored, it would limit confusion for sepgsql users if the documentation would say it at this point, e.g. If the installation process completes without error, you can now start the server normally. Warnings from errors in sepgsql_contexts, a file external to PostgreSQL, are harmless and can be ignored. The point of this patch is replacement of existing mechanism... So, it is not necessary to define a new policy for testing. Thanks for elaborating on this. The security label is something like user-id or ownership/object-acl in the default database access controls. It checks a relationship between user-id and ownership/object-acl of the target object. If this relationship allowed particular actions like 'select', 'update' or others, it shall be allowed when user requires these actions. In similar way, 'db_table:select' is a type of action; 'select' on table object, not an identifier of user or objects. SELinux defines a set of allowed actions (such as 'db_table:select') between a particular pair of security labels (such as 'staff_t' and 'sepgsql_table_t'). The pg_seclabel holds only security label of object being referenced. So, you should see /selinux/class/db_*/perms to see list of permissions defined in the security policy (but limited number of them are in use, now). The system's default security policy (selinux-policy package) defines all the necessary labeles, and access control rules between them. So, we never need to modify security policy to run regression test. The sepgsql_trusted_proc_exec_t means that functions labeled with this label is a trusted procedure. It switches security label of the user during execution of this function. It is a similar mechanism like SetExec or security definer function. The sepgsql_ro_table_t means 'read-only' tables that disallow any writer operations except for administrative domains. You can define your own policy, however, I intend to run regression test without any modification of the default security policy. Thank you for this clarification. I have some ideas of things that if they were in the documentation they'd helped me. Instead of seeking agreement on each item, I propose that I gather documentation additions in a patch later after the review, and leave it up to you guys whether to include them or not. regards, Yeb -- Yeb Havinga http://www.mgrid.net/ Mastering Medical 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] [v9.1] sepgsql - userspace access vector cache
/etc/selinux/targeted/contexts/sepgsql_contexts: line 33 has invalid object type db_blobs It is not an error, but just a notification to inform users that sepgsql_contexts file contains invalid lines. It is harmless, so we can ignore them. I don't think sepgsql.sgml should mention about this noise, because it purely come from the problem in libselinux and refpolicy; these are external packages from viewpoint of PostgreSQL. This is in contradiction with the current phrase in the documentation that's right after the sepgsql.sql loading: If the installation process completes without error, you can now start the server normally. IMHO if there are warnings that can be ignored, it would limit confusion for sepgsql users if the documentation would say it at this point, e.g. If the installation process completes without error, you can now start the server normally. Warnings from errors in sepgsql_contexts, a file external to PostgreSQL, are harmless and can be ignored. Indeed, it might be confusable to understand whether the installation got completed correctly, or not. So, I appended more descriptions about this messages, as follows: + para + Please note that you may see the following notifications depending on + the combination of a particular version of productnamelibselinux/ + and productnameselinux-policy/. +screen +/etc/selinux/targeted/contexts/sepgsql_contexts: line 33 has invalid object ty +/screen + It is harmless messages and already fixed. So, you can ignore these + messages or update related packages to the latest version. + /para See the attached patch, that contains other 3 documentation updates. Thank you for this clarification. I have some ideas of things that if they were in the documentation they'd helped me. Instead of seeking agreement on each item, I propose that I gather documentation additions in a patch later after the review, and leave it up to you guys whether to include them or not. OK, I like to check them. In addition, I'll also revise the wikipage in parallel to inform correctly. Thanks, -- NEC Europe Ltd, SAP Global Competence Center KaiGai Kohei kohei.kai...@emea.nec.com pgsql-sepgsql-doc-revise.2.patch Description: pgsql-sepgsql-doc-revise.2.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.1] sepgsql - userspace access vector cache
On 2011-07-19 12:10, Kohei Kaigai wrote: See the attached patch, that contains other 3 documentation updates. I looked at the patch and the additions look good, though I didn't actually apply it yet. thanks Yeb -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cascade replication
On Tue, Jul 19, 2011 at 5:58 PM, Simon Riggs si...@2ndquadrant.com wrote:
On Mon, Jul 11, 2011 at 7:28 AM, Fujii Masao masao.fu...@gmail.com wrote:
Attached is the updated version which addresses all the issues raised by
Simon.
Is there any reason why we disallow cascading unless hot standby is enabled?
ISTM we can just alter the postmaster path for walsenders, patch attached.
Some people might be happier if a sync standby were not HS enabled,
yet able to cascade to other standbys for reading.
- return CAC_STARTUP; /* normal startup */
+ {
+ if (am_walsender)
+ return CAC_OK;
+ else
+ return CAC_STARTUP; /* normal startup */
+ }
In canAcceptConnections(), am_walsender is always false, so the above CAC_OK
is never returned. You should change ProcessStartupPacket() as follows, instead.
switch (port-canAcceptConnections)
{
case CAC_STARTUP:
+ if (am_walsender)
+ {
+ port-canAcceptConnections = CAC_OK;
+ break;
+ }
ereport(FATAL,
When I fixed the above, compile the code and set up the cascading replication
environment (disable hot_standby), I got the following assertion error:
TRAP: FailedAssertion(!(slot 0 slot =
PMSignalState-num_child_flags), File: pmsignal.c, Line: 227)
So we would still have some code to change.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Function argument names in pg_catalog
Hi hackers, I'm curios why argument names (argname) are not used in the DDL for functions in pg_catalog, while they are are used throughout the documentation. For example, the documentation for pg_read_file in Table 9-60[1] has an SQL prototype: pg_read_file(filename text, offset bigint, length bigint) then why isn't the DDL for the function instead something like: CREATE OR REPLACE FUNCTION public.pg_read_file(filename text, offset bigint, length bigint) RETURNS text AS 'pg_read_file' LANGUAGE internal VOLATILE STRICT COST 1; There are two advantages for using argument names for function definitions: to add extra documentation for the parameters, and allow named notation (where applicable). For the extra documentation[2] point, the SQL prototype is visible in PgAdmin or psql. For example, with the above example try \df public.pg_read_file, the fourth column shows 'filename text, offset bigint, length bigint' in the fourth column. The existing \df pg_catalog.pg_read_file returns text, bigint, bigint, which sends the user to look up the function in the documentation to determine which bigint parameter is for length or offset. Having built-in extra documentation saves this trip. For the named notation[3] rational, a user can rearrange the arguments: select public.pg_read_file(offset := 200, length := 10, filename := 'myfile') or more practically, if parameters in the function were defined with default_expr, then the named parameters can be used while omitting default_expr parameters to accept defaults. Are there any drawbacks? Performance/bloat? Technical limitations? Apologies for my ignorance on how the DDL for functions in pg_catalog are defined. I can only assume they are generated from their internal C functions, as I can't find a pg_catalog.sql file in the source. Thanks for your thoughts, -Mike [1] http://www.postgresql.org/docs/current/static/functions-admin.html [2] http://www.postgresql.org/docs/current/static/sql-createfunction.html [3] http://www.postgresql.org/docs/current/static/sql-syntax-calling-funcs.html -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cascade replication
On Tue, Jul 19, 2011 at 12:19 PM, Fujii Masao masao.fu...@gmail.com wrote: So we would still have some code to change. Sigh, yes, of course. The question was whether there is any reason we need to disallow cascading? -- Simon Riggs 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] Cascade replication
On Tue, Jul 19, 2011 at 9:09 PM, Simon Riggs si...@2ndquadrant.com wrote: On Tue, Jul 19, 2011 at 12:19 PM, Fujii Masao masao.fu...@gmail.com wrote: So we would still have some code to change. Sigh, yes, of course. The question was whether there is any reason we need to disallow cascading? No, at least I have no clear reason for now. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cascade replication
On Tue, Jul 19, 2011 at 1:38 PM, Fujii Masao masao.fu...@gmail.com wrote: On Tue, Jul 19, 2011 at 9:09 PM, Simon Riggs si...@2ndquadrant.com wrote: On Tue, Jul 19, 2011 at 12:19 PM, Fujii Masao masao.fu...@gmail.com wrote: So we would still have some code to change. Sigh, yes, of course. The question was whether there is any reason we need to disallow cascading? No, at least I have no clear reason for now. I'll work up a proper patch. Thanks for your earlier review, -- Simon Riggs 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] storing TZ along timestamps
Josh Berkus j...@agliodbs.com wrote: The timestamp and the timezone in which that timestamp was entered are two separate pieces of data and *ought* to be in two separate fields. So, if you're grabbing a timestamp and the time zone for it, how do you ensure you've done that atomically if you're at the boundary of a DST change? The difficulty of grabbing both such that they are guaranteed to correspond suggests to me that they really form a single logical value. For one thing, the question of what timezone was this entered in is an application-specific question, since you have three different potential timezones: * the actual client timezone * the actual server timezone * the application timezone if the application has configurable timezones In a builtin data type, which of those three would you pick? Well clearly the only one *PostgreSQL* would pick is one assigned within the database server; otherwise, for a data type like this the value coming over the wire should specify it. If I want the client side value (in Java) it's easy enough to get such a value. new GregorianCalendar() is described thusly: | Constructs a default GregorianCalendar using the current time in | the default time zone with the default locale. How does Java assign those defaults? Why should PostgreSQL care? It's got the means to do so for itself. The point is, people can easily establish such a value on the client side; why not on the server side? Only the application knows [whether it should pick the value or let the database pick it]. When are things otherwise? Obviously the application will assign it or choose to let the server assign it (if that's the right thing). Additionally, if you have your timestamp-with-original-timezone data type, then you're going to need to recode every single timestamp-handling function and operator to handle the new type. Why? I think you'd want to add some *new* casts and operators for the new data type; I don't see why any existing ones would need to be modified. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Avoid index rebuilds for no-rewrite ALTER TABLE ALTER TYPE
On Tue, Jul 19, 2011 at 12:24 AM, Peter Eisentraut pete...@gmx.net wrote: Please review and fix this compiler warning: indexcmds.c: In function ‘CheckIndexCompatible’: indexcmds.c:126:15: warning: variable ‘amoptions’ set but not used [-Wunused-but-set-variable] I have removed the offending variable. -- 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] Commitfest Status: Sudden Death Overtime
On 2011-07-18 21:59, Robert Haas wrote: There are only two patches left and I think we really ought to try to take a crack at doing something with them. Yeb is working on the userspace access vector cache patch, which I think is going drag on longer than we want keep the CommitFest open, but I'm OK with giving it a day or two to shake out. At the end if this day I'm nearing the 'my head a splode' moment, which is more caused by trying to get my brain around selinux and it's postgresql policy, than the actual patch to review. I've verified that the patch works as indicated by KaiGai-san, but reading and understanding the code to say anything useful about it will take a few more hours, which will be tomorrow. regards, Yeb -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Exclude core dumps from project - example patch
=?ISO-8859-2?Q?pasman_pasma=F1ski?= pasma...@gmail.com writes: I am learn the git. For test my skills, here is patch to exclude core dumps from git tree. This doesn't seem like a particularly good idea. The project policy is to exclude only files that would normally appear during a build, and not cruft that might be generated by unexpected events. You might consider excluding such files in a personal ~/.gitconfig, if you would prefer to have git not tell you that your source tree is cluttered with them. See the core.excludesfile setting. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] storing TZ along timestamps
Kevin Grittner kevin.gritt...@wicourts.gov writes: Josh Berkus j...@agliodbs.com wrote: The timestamp and the timezone in which that timestamp was entered are two separate pieces of data and *ought* to be in two separate fields. So, if you're grabbing a timestamp and the time zone for it, how do you ensure you've done that atomically if you're at the boundary of a DST change? In my view of the world, the timezone that you are in is not an object that changes across a DST boundary. So the above is a red herring. It is only a problem if you insist on a broken concept of what a timezone is. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: a validator for configuration files
Peter Eisentraut pete...@gmx.net writes: On sön, 2011-07-17 at 00:59 -0400, Tom Lane wrote: Well, we *do* have a C API for that, of a sort. The problem is, what do you do in processes that have not loaded the relevant extension? Those processes that have the extension loaded check the parameter settings in their namespace, those that don't ignore them. Then you don't have any meaningful reporting of whether you have entered valid values --- particularly not with the policy that only the postmaster makes logfile entries about bad values. It'd work but I don't think it's tremendously user-friendly. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: a validator for configuration files
Josh Berkus j...@agliodbs.com writes: Hmmm. As someone who often deploys pg.conf changes as part of a production code rollout, I actually like the atomic nature of updating postgresql.conf -- that is, all your changes succeed, or they all fail. If we actually *had* that, I'd agree with you. The problem is that it appears that we have such a behavior, but it fails to work that way in corner cases. My proposal is aimed at making the corner cases less corner-y, by adopting a uniform rule that each backend adopts all the changes it can. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Single pass vacuum - take 1
Excerpts from Pavan Deolasee's message of lun jul 18 14:50:03 -0400 2011: On Mon, Jul 18, 2011 at 3:14 AM, Simon Riggs si...@2ndquadrant.com wrote: I will be happy to remove it again when we have shown there are no bugs getting this wrong is a data loss issue. Though I understand the fear for data loss, do we have much precedent of adding GUC to control such mechanism ? Even for complex feature like HOT we did not add any GUC to turn it off and I don't think we missed it. So I would suggest we review the code and test the feature extensively and fix the bugs if any, but lets not add any GUC to turn it off. In fact, the code and algorithm itself is not that complex and I would suggest you to take a look at the patch. Yeah. Having two implementations is much worse. We're going to have databases upgraded from previous versions that had the old behavior for a while and then switched (when pg_upgraded), and also databases that only have the new behavior. That's complex enough. If we add a GUC, we're going to have databases that ran with the new behavior for a while, then switched to the old one, and maybe back and forth a few times; debugging that kind of stuff is going to be interesting (for expensive values of interestingness). -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 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] Commitfest Status: Sudden Death Overtime
Robert Haas robertmh...@gmail.com writes: On Mon, Jul 18, 2011 at 4:19 PM, Tom Lane t...@sss.pgh.pa.us wrote: If you mean the business about allowing GUCs in postgresql.conf to be applied even if there are semantic errors elsewhere, I'm just as happy to let Alexey or Florian have a go at it first, if they want. The real question at the moment is do we have consensus about changing that? Because if we do, the submitted patch is certainly not something to commit as-is, and should be marked Returned With Feedback. I'm not totally convinced. The proposed patch is pretty small, and seems to stand on its own two feet. I don't hear anyone objecting to your proposed plan, but OTOH it doesn't strike me as such a good plan that we should reject all other improvements in the meantime. Maybe I'm missing something... To me, the proposed patch adds another layer of contortionism on top of code that's already logically messy. I find it pretty ugly, and would prefer to try to simplify the code before not after we attempt to deal with the feature the patch wants to add. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] storing TZ along timestamps
Tom Lane t...@sss.pgh.pa.us wrote: Kevin Grittner kevin.gritt...@wicourts.gov writes: Josh Berkus j...@agliodbs.com wrote: The timestamp and the timezone in which that timestamp was entered are two separate pieces of data and *ought* to be in two separate fields. So, if you're grabbing a timestamp and the time zone for it, how do you ensure you've done that atomically if you're at the boundary of a DST change? In my view of the world, the timezone that you are in is not an object that changes across a DST boundary. You're right -- the moment in time should be fixed like in the current PostgreSQL timestamp with time zone, and the time zone doesn't change with DST. Not an intentional read herring, but definitely some muddy thinking there. That weakens the argument for such a data type. Even with that, I suspect that its value as a convenience for application programmers would be sufficient that an extension to provide such functionality would get used. Essentially the current timestamptz bundled with a time zone and which is, by default, displayed at time zone of the attached time zone on output. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] storing TZ along timestamps
On 19 July 2011 17:11, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Tom Lane t...@sss.pgh.pa.us wrote: Kevin Grittner kevin.gritt...@wicourts.gov writes: Josh Berkus j...@agliodbs.com wrote: The timestamp and the timezone in which that timestamp was entered are two separate pieces of data and *ought* to be in two separate fields. So, if you're grabbing a timestamp and the time zone for it, how do you ensure you've done that atomically if you're at the boundary of a DST change? In my view of the world, the timezone that you are in is not an object that changes across a DST boundary. You're right -- the moment in time should be fixed like in the current PostgreSQL timestamp with time zone, and the time zone doesn't change with DST. Not an intentional read herring, but definitely some muddy thinking there. There was an earlier point made that if someone puts eg 5pm local time two years in the future into the database, and then the DST boundary gets moved subsequently, some applications would like the value to still say 5pm local time, even though that means it now refers to a different point in absolute time - this potentially seems like a useful feature. Retroactive timezone changes wouldn't make a lot of sense in this case though... I guess there are three concepts of time here - an absolute fixed time with no reference to a timezone, a time with a timezone that is still set as a fixed point in time, or a local time in a specific timezone that would move if the timezone definition changed. Ian -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest Status: Sudden Death Overtime
Excerpts from Tom Lane's message of mar jul 19 12:09:24 -0400 2011: Robert Haas robertmh...@gmail.com writes: On Mon, Jul 18, 2011 at 4:19 PM, Tom Lane t...@sss.pgh.pa.us wrote: If you mean the business about allowing GUCs in postgresql.conf to be applied even if there are semantic errors elsewhere, I'm just as happy to let Alexey or Florian have a go at it first, if they want. The real question at the moment is do we have consensus about changing that? Because if we do, the submitted patch is certainly not something to commit as-is, and should be marked Returned With Feedback. I'm not totally convinced. The proposed patch is pretty small, and seems to stand on its own two feet. I don't hear anyone objecting to your proposed plan, but OTOH it doesn't strike me as such a good plan that we should reject all other improvements in the meantime. Maybe I'm missing something... To me, the proposed patch adds another layer of contortionism on top of code that's already logically messy. I find it pretty ugly, and would prefer to try to simplify the code before not after we attempt to deal with the feature the patch wants to add. +1. Alexey stated that he would get back on this patch for reworks. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 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] FOR KEY LOCK foreign keys
Excerpts from Kevin Grittner's message of sáb jul 16 14:03:31 -0400 2011: Noah Misch wrote: With this patch in its final form, I have completed 180+ suite runs without a failure. The attached patch allows the tests to pass when default_transaction_isolation is stricter than 'read committed'. This is a slight change from the previously posted version of the files (because of a change in the order of statements, based on the timeouts), and in patch form this time. Thanks, applied. Sorry for the delay. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and log file output on Windows
Andrew Dunstan wrote: I can't figure out of there is something odd about this user's setup or if there is a bug in pg_upgrade with -l on Windows. The Windows file system seems to have some asynchronicity regarding what files are locked. For that reason, the buildfarm code has long had a couple of sleep(5) calls where it calls pg_ctl. You might benefit from doing something similar. Wow, I had no idea --- I can certainly add them. It is possible the person testing this has a faster machine than other users. I will report back after testing with a sleep(5). -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Function argument names in pg_catalog
Excerpts from Mike Toews's message of mar jul 19 07:23:24 -0400 2011: Hi hackers, I'm curios why argument names (argname) are not used in the DDL for functions in pg_catalog, while they are are used throughout the documentation. For example, the documentation for pg_read_file in Table 9-60[1] has an SQL prototype: pg_read_file(filename text, offset bigint, length bigint) then why isn't the DDL for the function instead something like: CREATE OR REPLACE FUNCTION public.pg_read_file(filename text, offset bigint, length bigint) RETURNS text AS 'pg_read_file' LANGUAGE internal VOLATILE STRICT COST 1; Probably mostly historical. We only got argument names (useful argument names) recently. Apologies for my ignorance on how the DDL for functions in pg_catalog are defined. I can only assume they are generated from their internal C functions, as I can't find a pg_catalog.sql file in the source. They are generated from pg_proc.h. I think we only have argument names for functions that have OUT arguments. See the pg_stat_file entry for an example. I'm not sure how open we are to adding names to more builtin functions. If catalog bloat is the only problem it'd cause, my guess is that it should be OK. I know that I have personally been bitten by not having argument names in builtin functions; they are pretty good run-time documentation. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 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] FOR KEY LOCK foreign keys
Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Kevin Grittner's message: Noah Misch wrote: With this patch in its final form, I have completed 180+ suite runs without a failure. The attached patch allows the tests to pass when default_transaction_isolation is stricter than 'read committed'. This is a slight change from the previously posted version of the files (because of a change in the order of statements, based on the timeouts), and in patch form this time. Thanks, applied. Sorry for the delay. My patch was intended to supplement Noah's patch here: http://archives.postgresql.org/pgsql-hackers/2011-07/msg00867.php Without his patch, there is still random failure on my work machine at all transaction isolation levels. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] FOR KEY LOCK foreign keys
Excerpts from Kevin Grittner's message of mar jul 19 13:49:53 -0400 2011: Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Kevin Grittner's message: Noah Misch wrote: With this patch in its final form, I have completed 180+ suite runs without a failure. The attached patch allows the tests to pass when default_transaction_isolation is stricter than 'read committed'. This is a slight change from the previously posted version of the files (because of a change in the order of statements, based on the timeouts), and in patch form this time. Thanks, applied. Sorry for the delay. My patch was intended to supplement Noah's patch here: I'm aware of that, thanks. I'm getting that one in too, shortly. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 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] include host names in hba error messages
On Tue, Jul 19, 2011 at 2:18 AM, Peter Eisentraut pete...@gmx.net wrote: Since we are accepting host names in pg_hba.conf now, I figured it could be useful to also show the host names in error message, e.g., no pg_hba.conf entry for host localhost (127.0.0.1), user x, database y Attached is an example patch. The question might be what criterion to use for when to show the host name. It could be if (port-remote_hostname_resolv == +1) that is, we have done the reverse and forward lookup, or if (port-remote_hostname_resolv = 0) that is, we have only done the reverse lookup (which is consistent with log_hostname). Although this whole thing could be quite weird, because the message that a host name was rejected because the forward lookup didn't match the IP address is at DEBUG2, so it's usually never shown. So if we tell someone that there is 'no pg_hba.conf entry for host foo', even though there is clearly a line saying foo in the file, it would be confusing. Ideas? I think it would be less confusing to write the IP address as the main piece of information, and put the hostname in parentheses only if we accepted it as valid (i.e. we did both lookups, and everything matched). ERROR: no pg_hba.conf entry for host 127.0.0.1 (localhost), user x, database y As for the case where we the forward lookup and reverse lookup don't match, could we add that as a DETAIL? ERROR: no pg_hba.conf entry for host 127.0.0.1, user x, database y DETAIL: Forward and reverse DNS lookups do not match. -- 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] FOR KEY LOCK foreign keys
Excerpts from Noah Misch's message of sáb jul 16 13:11:49 -0400 2011: In any event, I have attached a patch that fixes the problems I have described here. To ignore autovacuum, it only recognizes a wait when one of the backends under test holds a conflicting lock. (It occurs to me that perhaps we should expose a pg_lock_conflicts(lockmode_held text, lockmode_req text) function to simplify this query -- this is a fairly common monitoring need.) Applied it. I agree that having such an utility function is worthwhile, particularly if we're working on making pg_locks more usable as a whole. (I wasn't able to reproduce Rémi's hangups here, so I wasn't able to reproduce the other bits either.) With that change in place, my setup survived through about fifty suite runs at a time. The streak would end when session 2 would unexpectedly detect a deadlock that session 1 should have detected. The session 1 deadlock_timeout I chose, 20ms, is too aggressive. When session 2 is to issue the command that completes the deadlock, it must do so before session 1 runs the deadlock detector. Since we burn 10ms just noticing that the previous statement has blocked, that left only 10ms to issue the next statement. This patch bumps the figure from 20s to 100ms; hopefully that will be enough for even a decently-loaded virtual host. Committed this too. With this patch in its final form, I have completed 180+ suite runs without a failure. In the absence of better theories on the cause for the buildfarm failures, we should give the buildfarm a whirl with this patch. Great. If there is some other failure mechanism, we'll find out ... I apologize for the quantity of errata this change is entailing. No need to apologize. I might as well apologize myself because I didn't detect these problems on review. But we don't do that -- we just fix the problems and move on. It's great that you were able to come up with a fix quickly. And this is precisely why I committed this way ahead of the patch that it was written to help: we're now not fixing problems in both simultaneously. By the time we get that other patch in, this test harness will be fully robust. Thanks for all your effort in this. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and log file output on Windows
On 07/19/2011 01:25 PM, Bruce Momjian wrote: Andrew Dunstan wrote: I can't figure out of there is something odd about this user's setup or if there is a bug in pg_upgrade with -l on Windows. The Windows file system seems to have some asynchronicity regarding what files are locked. For that reason, the buildfarm code has long had a couple of sleep(5) calls where it calls pg_ctl. You might benefit from doing something similar. Wow, I had no idea --- I can certainly add them. It is possible the person testing this has a faster machine than other users. I will report back after testing with a sleep(5). We need to work out a way to get pg_upgrade testing into the buildfarm. It's becoming too important not to. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] A few user-level questions on Streaming Replication and pg_upgrade
Hi, Here are a few questions that were asked by a customer, who are trying to assess the pros and cons of using Postgres and its SR feature. I would like to get an opinion of someone more involved with the community than me. .) Will Postgres support Streaming Replication from 9.0.x to 9.1.x; i.e. across major releases. I am pretty sure the answer is no, it won't, but just double-checking with the community. .) Is Streaming Replication supported across minor releases, in reverse direction; e.g. 9.0.3 to 9.0.1 I think the answer is it depends, since it would depend upon whether any SR related bug has been fixed in the 'greater' of the minor releases. I am assuming that smaller minor release to bigger minor release will always be supported (e.g. 9.0.1 to 9.0.3) .) How reliable is `pg_upgrade -c` (dry run) currently; that is, how accurate is pg_upgrade at predicting any potential problem with the eventual in-place upgrade. I'd say it is as reliable as it gets since this is the official tool supported by the project, and it should not contain any known bugs. One has to use the latest and greatest 'minor' version of the tool for the major release they are upgrading to, though. I'd also like to mention a piece of information that may be surprising to some. Per Tom at a PGCon dinner, Postgres project does not promise continued guarantee of in-place upgrades across future major releases. Although the project will try hard to avoid having to make any changes that may affect in-place upgrade capability, but if a case can be made that a feature would give a significant improvement at the cost of compromising this capability, then the in-place upgrade capability may be forgone for that release. Thanks in advance, -- Gurjeet Singh EnterpriseDB Corporation The Enterprise PostgreSQL Company
Re: [HACKERS] proposal: new contrib module plpgsql's embeded sql validator
On 07/17/2011 10:31 PM, Jim Nasby wrote: On Jul 7, 2011, at 11:31 PM, Pavel Stehule wrote: a lazy deep SQL validation inside plpgsq functions is interesting attribute. It allows to work with temporary tables and it make testing and debugging harder, because lot of errors in embedded queries are detected too late. I wrote a simple module that can to help little bit. It is based on plpgsql plugin API and it ensures a deep validation of embedded sql early - after start of execution. I am thinking, so this plugin is really useful and it is example of plpgsql pluging - that is missing in contrib. I think this should at least be a contrib module; it seems very useful. Yes I agree this should be part of pg distribution. But, I think we should add valitation hook to plpgsql plugin structure so that you don't have to actually execute the function to check it - curretly there are only executing hooks which is why the plugin only works when you the func (not good for automation). -- Petr Jelinek -- Sent 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: new contrib module plpgsql's embeded sql validator
Dne 19. července 2011 21:15 Petr Jelínek pjmo...@pjmodos.net napsal(a): On 07/17/2011 10:31 PM, Jim Nasby wrote: On Jul 7, 2011, at 11:31 PM, Pavel Stehule wrote: a lazy deep SQL validation inside plpgsq functions is interesting attribute. It allows to work with temporary tables and it make testing and debugging harder, because lot of errors in embedded queries are detected too late. I wrote a simple module that can to help little bit. It is based on plpgsql plugin API and it ensures a deep validation of embedded sql early - after start of execution. I am thinking, so this plugin is really useful and it is example of plpgsql pluging - that is missing in contrib. I think this should at least be a contrib module; it seems very useful. Yes I agree this should be part of pg distribution. But, I think we should add valitation hook to plpgsql plugin structure so that you don't have to actually execute the function to check it - curretly there are only executing hooks which is why the plugin only works when you the func (not good for automation). should be great, but there are still few limits in compile time * polymorphic parameters * triggers - there are no a info about relation in compile time we can adapt a #option keyword for using in some plpgsql plugins for example - for addition information that are necessary for usage of lint in compilation time CREATE OR REPLACE FUNCTION foo () RETURNS ... AS $$ #option trigger_relation some_table_name #option replace_anyelement integer ... with this addition info it and some compile hook it is possible Regards Pavel -- Petr Jelinek -- 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: [COMMITTERS] pgsql: Remove O(N^2) performance issue with multiple SAVEPOINTs.
On 19.07.2011 19:22, Simon Riggs wrote: Remove O(N^2) performance issue with multiple SAVEPOINTs. Subtransaction locks now released en masse at main commit, rather than repeatedly re-scanning for locks as we ascend the nested transaction tree. Split transaction state TBLOCK_SUBEND into two states, TBLOCK_SUBCOMMIT and TBLOCK_SUBRELEASE to allow the commit path to be optimised using the existing code in ResourceOwnerRelease() which appears to have been intended for this usage, judging from comments therein. CommitSubTransaction(true) does this: ResourceOwnerRelease(s-curTransactionOwner, RESOURCE_RELEASE_LOCKS, true, isTopLevel /* == true */); ... ResourceOwnerDelete(s-curTransactionOwner); Because isTopLevel is passed as true, ResourceOwnerRelease() doesn't release or transfer the locks belonging to the resource owner. After the call, they still point to s-curTransactionOwner. Then, the resource owner is deleted. After those two calls, the locks still have pointers to the now-pfree'd ResourceOwner object. Looking at lock.c, we apparently never dereference LOCALLOCKOWNER.owner field. Nevertheless, a dangling pointer like that seems like a recipe for trouble. After releasing all subtransactions, we still fire deferred triggers, for example, which can do arbitrarily complex things. For example, you might allocate new resource owners, which if you're really unlucky might get allocated at the same address as the already-pfree'd resource owner. I'm not sure what would happen then, but it can't be good. Instead of leaving the locks dangling to an already-destroyed resource owner, how about assigning all locks directly to the top-level resource owner in one sweep? That'd still be much better than the old way of recursively reassigning them up the subtransaction tree, one level at a time. -- Heikki Linnakangas 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] [COMMITTERS] pgsql: Remove O(N^2) performance issue with multiple SAVEPOINTs.
On Tue, Jul 19, 2011 at 8:49 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 19.07.2011 19:22, Simon Riggs wrote: Remove O(N^2) performance issue with multiple SAVEPOINTs. Subtransaction locks now released en masse at main commit, rather than repeatedly re-scanning for locks as we ascend the nested transaction tree. Split transaction state TBLOCK_SUBEND into two states, TBLOCK_SUBCOMMIT and TBLOCK_SUBRELEASE to allow the commit path to be optimised using the existing code in ResourceOwnerRelease() which appears to have been intended for this usage, judging from comments therein. CommitSubTransaction(true) does this: ResourceOwnerRelease(s-curTransactionOwner, RESOURCE_RELEASE_LOCKS, true, isTopLevel /* == true */); ... ResourceOwnerDelete(s-curTransactionOwner); Because isTopLevel is passed as true, ResourceOwnerRelease() doesn't release or transfer the locks belonging to the resource owner. After the call, they still point to s-curTransactionOwner. Then, the resource owner is deleted. After those two calls, the locks still have pointers to the now-pfree'd ResourceOwner object. Looking at lock.c, we apparently never dereference LOCALLOCKOWNER.owner field. Nevertheless, a dangling pointer like that seems like a recipe for trouble. After releasing all subtransactions, we still fire deferred triggers, for example, which can do arbitrarily complex things. For example, you might allocate new resource owners, which if you're really unlucky might get allocated at the same address as the already-pfree'd resource owner. I'm not sure what would happen then, but it can't be good. Instead of leaving the locks dangling to an already-destroyed resource owner, how about assigning all locks directly to the top-level resource owner in one sweep? That'd still be much better than the old way of recursively reassigning them up the subtransaction tree, one level at a time. Yes, I did see what the code was doing. My feeling was the code was specifically written that way, just never used. So I wired it up to be used the way intended. Have a look at ResourceOwnerReleaseInternal()... not code I wrote or touched on this patch. You might persuade me to do it another way, but I can't see how to make that way work. Your case seems a stretch. Not sure why you mention it now, 7 weeks after review. -- Simon Riggs 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
[HACKERS] Re: [COMMITTERS] pgsql: Remove O(N^2) performance issue with multiple SAVEPOINTs.
On 19.07.2011 23:08, Simon Riggs wrote: On Tue, Jul 19, 2011 at 8:49 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 19.07.2011 19:22, Simon Riggs wrote: Remove O(N^2) performance issue with multiple SAVEPOINTs. Subtransaction locks now released en masse at main commit, rather than repeatedly re-scanning for locks as we ascend the nested transaction tree. Split transaction state TBLOCK_SUBEND into two states, TBLOCK_SUBCOMMIT and TBLOCK_SUBRELEASE to allow the commit path to be optimised using the existing code in ResourceOwnerRelease() which appears to have been intended for this usage, judging from comments therein. CommitSubTransaction(true) does this: ResourceOwnerRelease(s-curTransactionOwner, RESOURCE_RELEASE_LOCKS, true, isTopLevel /* == true */); ... ResourceOwnerDelete(s-curTransactionOwner); Because isTopLevel is passed as true, ResourceOwnerRelease() doesn't release or transfer the locks belonging to the resource owner. After the call, they still point to s-curTransactionOwner. Then, the resource owner is deleted. After those two calls, the locks still have pointers to the now-pfree'd ResourceOwner object. Looking at lock.c, we apparently never dereference LOCALLOCKOWNER.owner field. Nevertheless, a dangling pointer like that seems like a recipe for trouble. After releasing all subtransactions, we still fire deferred triggers, for example, which can do arbitrarily complex things. For example, you might allocate new resource owners, which if you're really unlucky might get allocated at the same address as the already-pfree'd resource owner. I'm not sure what would happen then, but it can't be good. Instead of leaving the locks dangling to an already-destroyed resource owner, how about assigning all locks directly to the top-level resource owner in one sweep? That'd still be much better than the old way of recursively reassigning them up the subtransaction tree, one level at a time. Yes, I did see what the code was doing. My feeling was the code was specifically written that way, just never used. So I wired it up to be used the way intended. Have a look at ResourceOwnerReleaseInternal()... not code I wrote or touched on this patch. The way ResourceOwnerReleaseIntenal(isTopLevel==true) works in the case of a genuine top-level commit doesn't have this problem, because the sub-resource owners are not deleted until TopTransactionResourceOwner has been processed, and all the locks released. In fact, before this patch I think an Assert(!isTopLevel || owner == TopTransactionResourceOwner) would've be in order in ResourceOwnerRelease(). Or it could've done bool isTopLevel = (owner == TopTransactionResoureOwner) in the beginning instead of having isTopLevel as an argument. You might persuade me to do it another way, but I can't see how to make that way work. Your case seems a stretch. You get coincidences with memory allocations surprisingly often, because things tend to get allocated and free'd in chunks of certain sizes. It's also pretty fragile in the face of future development. It's not hard to imagine someone adding code in lock.c to dereference the pointer. Not sure why you mention it now,7 weeks after review. Because I only just spotted it. -- Heikki Linnakangas 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] [v9.1] sepgsql - userspace access vector cache
On 19.07.2011 12:28, Yeb Havinga wrote: On 2011-07-18 22:21, Kohei KaiGai wrote: The Scientific Linux 6 is not suitable, because its libselinux version is a bit older than this patch expects (libselinux-2.0.99 or later). My recommendation is Fedora 15, instead. Installing right now, thanks for the heads up! Would it be reasonable to #ifdefs the parts that require version 2.0.99? That's very recent so might not be available on popular distributions for some time, so it would be nice to not have a hard dependency on it. You could have autoconf rules to check for the new functions, and only use them if they are available. -- Heikki Linnakangas 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] [COMMITTERS] pgsql: Remove O(N^2) performance issue with multiple SAVEPOINTs.
On Tue, Jul 19, 2011 at 9:24 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: You might persuade me to do it another way, but I can't see how to make that way work. Your case seems a stretch. You get coincidences with memory allocations surprisingly often, because things tend to get allocated and free'd in chunks of certain sizes. It's also pretty fragile in the face of future development. It's not hard to imagine someone adding code in lock.c to dereference the pointer. Then I think we need a 4th phase (would actually happen first). I will revoke and rework. -- Simon Riggs 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] [v9.1] sepgsql - userspace access vector cache
On 2011-07-19 22:39, Heikki Linnakangas wrote: On 19.07.2011 12:28, Yeb Havinga wrote: On 2011-07-18 22:21, Kohei KaiGai wrote: The Scientific Linux 6 is not suitable, because its libselinux version is a bit older than this patch expects (libselinux-2.0.99 or later). My recommendation is Fedora 15, instead. Installing right now, thanks for the heads up! Would it be reasonable to #ifdefs the parts that require version 2.0.99? That's very recent so might not be available on popular distributions for some time, so it would be nice to not have a hard dependency on it. You could have autoconf rules to check for the new functions, and only use them if they are available. In contrary to the subject I was under the impression the current patch is for the 9.2 release since it is in a commitfest for the 9.2 release cycle, which would make the libselinux-2.0.99 dependency less of a problem. -- Yeb Havinga http://www.mgrid.net/ Mastering Medical 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] storing TZ along timestamps
Alvaro, Kevin, In a builtin data type, which of those three would you pick? Only the application knows. I think this whole discussion is built on the assumption that the client timezone and the application timezone are one thing and the same; and the server timezone is not relevant at all. If the app TZ is not the client TZ, then the app will need fixed. Not at all. Consider a hosted webapp where the user is allowed to set their own timezone, but you use pooled connections. In that case, the app is going to be handling user timezones with an AT TIME ZONE, not with a SET TIMEZONE= I have my doubts about that, and I hope not. These details haven't been discussed at all; I only started this thread to get community approval on cataloguing the TZs. I am strongly in favor of having a *timezone* data type and some system whereby we can uniquely identify timezones in the Zic database. That would be tremendously useful for all sorts of things. I'm just asserting that those who want a composite timestamp+saved-time-zone data type have not thought about all of the complications involved. So, if you're grabbing a timestamp and the time zone for it, how do you ensure you've done that atomically if you're at the boundary of a DST change? The difficulty of grabbing both such that they are guaranteed to correspond suggests to me that they really form a single logical value. Not relevant, given that (hopefully) the conception of a time zone should exist independantly of whether it's currently in DST or not. That is, the time zone is NOT -07. The time zone is US/Pacific. Why? I think you'd want to add some *new* casts and operators for the new data type; I don't see why any existing ones would need to be modified. That would work too. What I'm pointing out is that we can't implement the new type using just one-line modifications to the old operators and functions. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] storing TZ along timestamps
On Jul 19, 2011, at 2:06 PM, Josh Berkus wrote: I am strongly in favor of having a *timezone* data type and some system whereby we can uniquely identify timezones in the Zic database. CREATE OR REPLACE FUNCTION is_timezone( tz CITEXT ) RETURNS BOOLEAN LANGUAGE plpgsql STABLE AS $$ BEGIN PERFORM NOW() AT TIME ZONE tz; RETURN TRUE; EXCEPTION WHEN invalid_parameter_value THEN RETURN FALSE; END; $$; CREATE DOMAIN timezone AS CITEXT CHECK ( is_timezone( VALUE ) ); Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] PgWest CFP closes in two weeks
Hey folks, As a reminder, PgWest is in a few months and the CFP closes in two weeks. Get those talks in! https://www.postgresqlconference.org/talk_types Sincerely, Joshua D. Drake -- Command Prompt, Inc. - http://www.commandprompt.com/ PostgreSQL Support, Training, Professional Services and Development The PostgreSQL Conference - http://www.postgresqlconference.org/ @cmdpromptinc - @postgresconf - 509-416-6579 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another issue with invalid XML values
Florian Pflug f...@phlo.org writes: Updated patch attached. Do you think this is Ready for Committer? I've been looking through this patch. While it's mostly good, I'm pretty unhappy with the way that the pg_xml_init/pg_xml_done code is deliberately designed to be non-reentrant (ie, throw an Assert if pg_xml_init is called twice without pg_xml_done in between). There are at least two issues with that: 1. If you forget pg_xml_done in some code path, you'll find out from an Assert at the next pg_xml_init, which is probably far away from where the actual problem is. 2. I don't think it's entirely unlikely that uses of libxml could be nested. xpath_table in particular calls an awful lot of stuff between pg_xml_init and pg_xml_done, and is at the very least open to loss of control via an elog before it's called pg_xml_done. I think this patch has already paid 90% of the notational price for supporting fully re-entrant use of libxml. What I'm imagining is that we move all five of the static variables (xml_strictness, xml_err_occurred, xml_err_buf, xml_structuredErrorFunc_saved, xml_structuredErrorContext_saved) into a struct that's palloc'd by pg_xml_init and eventually passed to pg_xml_done. It could be passed to xml_errorHandler via the currently-unused context argument. A nice side benefit is that we could get rid of PG_XML_STRICTNESS_NONE. Now the risk factor if we do that is that if someone misses a pg_xml_done call, we leave an error handler installed with a context argument that's probably pointing at garbage, and if someone then tries to use libxml without re-establishing their error handler, they've got problems. But they'd have problems anyway with the current form of the patch. We could provide some defense against this by including a magic identifier value in the palloc'd struct and making xml_errorHandler check it before doing anything dangerous. Also, we could make pg_xml_done warn if libxml's current context pointer is different from the struct passed to it, which would provide another means of detection that somebody had missed a cleanup call. Unless someone sees a major hole in this idea, or a better way to do it, I'm going to modify the patch along those lines and commit. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] A few user-level questions on Streaming Replication and pg_upgrade
Gurjeet Singh wrote: Hi, Here are a few questions that were asked by a customer, who are trying to assess the pros and cons of using Postgres and its SR feature. I would like to get an opinion of someone more involved with the community than me. .) Will Postgres support Streaming Replication from 9.0.x to 9.1.x; i.e. across major releases. I am pretty sure the answer is no, it won't, but just double-checking with the community. [ CC to general removed --- emailing only hackers; cross-posting is frowned upon. ] Right. .) Is Streaming Replication supported across minor releases, in reverse direction; e.g. 9.0.3 to 9.0.1 I think the answer is it depends, since it would depend upon whether any SR related bug has been fixed in the 'greater' of the minor releases. I am assuming that smaller minor release to bigger minor release will always be supported (e.g. 9.0.1 to 9.0.3) Yes. We could mention in the minor release notes if we break streaming replication for a minor release --- or someone will tell us when we do. .) How reliable is `pg_upgrade -c` (dry run) currently; that is, how accurate is pg_upgrade at predicting any potential problem with the eventual in-place upgrade. I'd say it is as reliable as it gets since this is the official tool supported by the project, and it should not contain any known bugs. One has to use the latest and greatest 'minor' version of the tool for the major release they are upgrading to, though. Well, we make no guarantees about the software at all, so it is hard to make any guarantee about pg_upgrade either. I'd also like to mention a piece of information that may be surprising to some. Per Tom at a PGCon dinner, Postgres project does not promise continued guarantee of in-place upgrades across future major releases. Although the project will try hard to avoid having to make any changes that may affect in-place upgrade capability, but if a case can be made that a feature would give a significant improvement at the cost of compromising this capability, then the in-place upgrade capability may be forgone for that release. Doesn't surprise me --- I know a time will come when we must change the data format enough to break pg_upgrade's ability to perform major upgrades. It is not 'if', but 'when'. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
On Mon, Jul 18, 2011 at 7:36 PM, Florian Pflug f...@phlo.org wrote: On Jul19, 2011, at 00:17 , Joey Adams wrote: I suppose a simple solution would be to convert all escapes and outright ban escapes of characters not in the database encoding. +1. Making JSON work like TEXT when it comes to encoding issues makes this all much simpler conceptually. It also avoids all kinds of weird issues if you extract textual values from a JSON document server-side. Thanks for the input. I'm leaning in this direction too. However, it will be a tad tricky to implement the conversions efficiently, since the wchar API doesn't provide a fast path for individual codepoint conversion (that I'm aware of), and pg_do_encoding_conversion doesn't look like a good thing to call lots of times. My plan is to scan for escapes of non-ASCII characters, convert them to UTF-8, and put them in a comma-delimited string like this: a,b,c,d, then, convert the resulting string to the server encoding (which may fail, indicating that some codepoint(s) are not present in the database encoding). After that, read the string and plop the characters where they go. It's clever, but I can't think of a better way to do it with the existing API. - Joey -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another issue with invalid XML values
Excerpts from Tom Lane's message of mar jul 19 19:42:54 -0400 2011: Now the risk factor if we do that is that if someone misses a pg_xml_done call, we leave an error handler installed with a context argument that's probably pointing at garbage, and if someone then tries to use libxml without re-establishing their error handler, they've got problems. But they'd have problems anyway with the current form of the patch. We could provide some defense against this by including a magic identifier value in the palloc'd struct and making xml_errorHandler check it before doing anything dangerous. Also, we could make pg_xml_done warn if libxml's current context pointer is different from the struct passed to it, which would provide another means of detection that somebody had missed a cleanup call. Unless someone sees a major hole in this idea, or a better way to do it, I'm going to modify the patch along those lines and commit. I don't see any holes in this idea (though I didn't look very hard), but I was thinking that maybe it's time for this module to hook onto the cleanup stuff for the xact error case; or at least have a check that it has been properly cleaned up elesewhere. Maybe this can be made to work reentrantly if there's a global var holding the current context, and it contains a link to the next one up the stack. At least, my impression is that the PG_TRY blocks are already messy. BTW I'd like to know your opinion on the fact that this patch added two new StringInfo routines defined as static in xml.c. It seems to me that if we're going to extend some module's API we should do it properly in its own files; otherwise we're bound to repeat the functionality elsewhere, and lose opportunities for cleaning up some other code that could presumably use similar functionality. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 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: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
Excerpts from Joey Adams's message of mar jul 19 21:03:15 -0400 2011: On Mon, Jul 18, 2011 at 7:36 PM, Florian Pflug f...@phlo.org wrote: On Jul19, 2011, at 00:17 , Joey Adams wrote: I suppose a simple solution would be to convert all escapes and outright ban escapes of characters not in the database encoding. +1. Making JSON work like TEXT when it comes to encoding issues makes this all much simpler conceptually. It also avoids all kinds of weird issues if you extract textual values from a JSON document server-side. Thanks for the input. I'm leaning in this direction too. However, it will be a tad tricky to implement the conversions efficiently, since the wchar API doesn't provide a fast path for individual codepoint conversion (that I'm aware of), and pg_do_encoding_conversion doesn't look like a good thing to call lots of times. My plan is to scan for escapes of non-ASCII characters, convert them to UTF-8, and put them in a comma-delimited string like this: a,b,c,d, then, convert the resulting string to the server encoding (which may fail, indicating that some codepoint(s) are not present in the database encoding). After that, read the string and plop the characters where they go. Ugh. It's clever, but I can't think of a better way to do it with the existing API. Would it work to have a separate entry point into mbutils.c that lets you cache the conversion proc caller-side? I think the main problem is determining the byte length of each source character beforehand. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 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] range types and ip4r
On Tue, 2011-07-19 at 09:38 +0300, Peter Eisentraut wrote: Just wondering, will the planned range type functionality also be able to absorb the functionality of the ip4r type as a range of the ip4 type (http://pgfoundry.org/projects/ip4r)? Maybe it's trivial, but since the ip types also have a kind of hierarchical structure, I figured I'd point it out in case you hadn't considered it. Thanks for bringing that up. It had briefly crossed my mind, but I didn't see any problem with it. Does it use the hierarchical nature to manipulate the values at all, or is it just a flat range? If it's just a flat range it would be similar to int4range, I would think. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Fwd: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
Forwarding because the mailing list rejected the original message. -- Forwarded message -- From: Joey Adams joeyadams3.14...@gmail.com Date: Tue, Jul 19, 2011 at 11:23 PM Subject: Re: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON To: Alvaro Herrera alvhe...@commandprompt.com Cc: Florian Pflug f...@phlo.org, Tom Lane t...@sss.pgh.pa.us, Robert Haas robertmh...@gmail.com, Bernd Helmle maili...@oopsware.de, Dimitri Fontaine dimi...@2ndquadrant.fr, David Fetter da...@fetter.org, Josh Berkus j...@agliodbs.com, Pg Hackers pgsql-hackers@postgresql.org On Tue, Jul 19, 2011 at 10:01 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Would it work to have a separate entry point into mbutils.c that lets you cache the conversion proc caller-side? That sounds like a really good idea. There's still the overhead of calling the proc, but I imagine it's a lot less than looking it up. I think the main problem is determining the byte length of each source character beforehand. I'm not sure what you mean. The idea is to convert the \u escape to UTF-8 with unicode_to_utf8 (the length of the resulting UTF-8 sequence is easy to compute), call the conversion proc to get the null-terminated database-encoded character, then append the result to whatever StringInfo the string is going into. The only question mark is how big the destination buffer will need to be. The maximum number of bytes per char in any supported encoding is 4, but is it possible for one Unicode character to turn into multiple characters in the database encoding? While we're at it, should we provide the same capability to the SQL parser? Namely, the ability to use \u escapes above U+007F when the server encoding is not UTF-8? - Joey -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Fwd: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
Joey Adams wrote: Forwarding because the mailing list rejected the original message. Yes, I am seeing email failures to the 'core' email list. --- -- Forwarded message -- From: Joey Adams joeyadams3.14...@gmail.com Date: Tue, Jul 19, 2011 at 11:23 PM Subject: Re: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON To: Alvaro Herrera alvhe...@commandprompt.com Cc: Florian Pflug f...@phlo.org, Tom Lane t...@sss.pgh.pa.us, Robert Haas robertmh...@gmail.com, Bernd Helmle maili...@oopsware.de, Dimitri Fontaine dimi...@2ndquadrant.fr, David Fetter da...@fetter.org, Josh Berkus j...@agliodbs.com, Pg Hackers pgsql-hackers@postgresql.org On Tue, Jul 19, 2011 at 10:01 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Would it work to have a separate entry point into mbutils.c that lets you cache the conversion proc caller-side? That sounds like a really good idea. ?There's still the overhead of calling the proc, but I imagine it's a lot less than looking it up. I think the main problem is determining the byte length of each source character beforehand. I'm not sure what you mean. ?The idea is to convert the \u escape to UTF-8 with unicode_to_utf8 (the length of the resulting UTF-8 sequence is easy to compute), call the conversion proc to get the null-terminated database-encoded character, then append the result to whatever StringInfo the string is going into. The only question mark is how big the destination buffer will need to be. ?The maximum number of bytes per char in any supported encoding is 4, but is it possible for one Unicode character to turn into multiple characters in the database encoding? While we're at it, should we provide the same capability to the SQL parser? ?Namely, the ability to use \u escapes above U+007F when the server encoding is not UTF-8? - Joey -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Fwd: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
Bruce Momjian wrote: Joey Adams wrote: Forwarding because the mailing list rejected the original message. Yes, I am seeing email failures to the 'core' email list. Marc says it is now fixed. --- -- Forwarded message -- From: Joey Adams joeyadams3.14...@gmail.com Date: Tue, Jul 19, 2011 at 11:23 PM Subject: Re: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON To: Alvaro Herrera alvhe...@commandprompt.com Cc: Florian Pflug f...@phlo.org, Tom Lane t...@sss.pgh.pa.us, Robert Haas robertmh...@gmail.com, Bernd Helmle maili...@oopsware.de, Dimitri Fontaine dimi...@2ndquadrant.fr, David Fetter da...@fetter.org, Josh Berkus j...@agliodbs.com, Pg Hackers pgsql-hackers@postgresql.org On Tue, Jul 19, 2011 at 10:01 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Would it work to have a separate entry point into mbutils.c that lets you cache the conversion proc caller-side? That sounds like a really good idea. ?There's still the overhead of calling the proc, but I imagine it's a lot less than looking it up. I think the main problem is determining the byte length of each source character beforehand. I'm not sure what you mean. ?The idea is to convert the \u escape to UTF-8 with unicode_to_utf8 (the length of the resulting UTF-8 sequence is easy to compute), call the conversion proc to get the null-terminated database-encoded character, then append the result to whatever StringInfo the string is going into. The only question mark is how big the destination buffer will need to be. ?The maximum number of bytes per char in any supported encoding is 4, but is it possible for one Unicode character to turn into multiple characters in the database encoding? While we're at it, should we provide the same capability to the SQL parser? ?Namely, the ability to use \u escapes above U+007F when the server encoding is not UTF-8? - Joey -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another issue with invalid XML values
[ resend due to mail server hiccup ] Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Tom Lane's message of mar jul 19 19:42:54 -0400 2011: Now the risk factor if we do that is that if someone misses a pg_xml_done call, we leave an error handler installed with a context argument that's probably pointing at garbage, and if someone then tries to use libxml without re-establishing their error handler, they've got problems. I don't see any holes in this idea (though I didn't look very hard), but I was thinking that maybe it's time for this module to hook onto the cleanup stuff for the xact error case; or at least have a check that it has been properly cleaned up elesewhere. Maybe this can be made to work reentrantly if there's a global var holding the current context, and it contains a link to the next one up the stack. At least, my impression is that the PG_TRY blocks are already messy. Yeah, that's another way we could go. But I'm not sure how well it would interact with potential third-party modules setting up their own libxml error handlers. Anybody have a thought about that? BTW I'd like to know your opinion on the fact that this patch added two new StringInfo routines defined as static in xml.c. It seems to me that if we're going to extend some module's API we should do it properly in its own files; otherwise we're bound to repeat the functionality elsewhere, and lose opportunities for cleaning up some other code that could presumably use similar functionality. I did think about that for a little bit, but the functions in question are only a couple lines long and seem rather specialized to what xml.c needs. I'd just as soon leave them as-is until we actually have a second use-case to help with picking a generalized API. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: new contrib module plpgsql's embeded sql validator
2011/7/20 Tom Lane t...@sss.pgh.pa.us: =?ISO-8859-1?Q?Petr_Jel=EDnek?= pjmo...@pjmodos.net writes: But, I think we should add valitation hook to plpgsql plugin structure so that you don't have to actually execute the function to check it - curretly there are only executing hooks which is why the plugin only works when you the func (not good for automation). If you mean that such checks would be done automatically, no, they shouldn't be. Consider a function that creates a table and then uses it, or even just depends on using a table that doesn't yet exist when you do CREATE FUNCTION. yes, any deep check is not possible for function that uses a temporary tables. A plpgsql_lint is not silver bullet - for these cases is necessary to disable lint. . I can't to speak generally - I have no idea, how much percent of functions are functions with access to temporary tables - in my last project I use 0 temp tables on cca 300 KB of plpgsql code. The more terrible problem is a new dependency between functions. I use a workaround - some like headers CREATE FUNCTIONS foo(define interface here) RETURNS ... AS $$ BEGIN RETURN; END; $$ LANGUAGE plpgsql; ... --real implementation of foo CREATE OR REPLACE FUNCTIONS foo(...) RETURNS ... AS .. It works because I write a plpgsql script in hand - I don't use a dump for plpgsql, but it is not solution for production servers. On second hand - plpgsql_lint or some similar (and builtin or external) should not be active on production servers. A planning only really processed queries is necessary optimization if we have not a global plan cache. Regards Pavel regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
On Tue, Jul 19, 2011 at 9:03 PM, Joey Adams joeyadams3.14...@gmail.com wrote: On Mon, Jul 18, 2011 at 7:36 PM, Florian Pflug f...@phlo.org wrote: On Jul19, 2011, at 00:17 , Joey Adams wrote: I suppose a simple solution would be to convert all escapes and outright ban escapes of characters not in the database encoding. +1. Making JSON work like TEXT when it comes to encoding issues makes this all much simpler conceptually. It also avoids all kinds of weird issues if you extract textual values from a JSON document server-side. Thanks for the input. I'm leaning in this direction too. However, it will be a tad tricky to implement the conversions efficiently, ... I'm a bit confused, because I thought what I was talking about was not doing any conversions in the first place. -- 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: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
On Wed, Jul 20, 2011 at 12:32 AM, Robert Haas robertmh...@gmail.com wrote: Thanks for the input. I'm leaning in this direction too. However, it will be a tad tricky to implement the conversions efficiently, ... I'm a bit confused, because I thought what I was talking about was not doing any conversions in the first place. We want to be able to handle \u escapes when the database encoding is not UTF-8. We could leave them in place, but sooner or later they'll need to be converted in order to unwrap or compare JSON strings. The approach being discussed is converting escapes to the database encoding. This means escapes of characters not available in the database encoding (e.g. \u266B in ISO-8859-1) will be forbidden. The PostgreSQL parser (which also supports Unicode escapes) takes a simpler approach: don't allow non-ASCII escapes unless the server encoding is UTF-8. - Joey -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
