Re: [HACKERS] [PATCH] Largeobject access controls
2009/9/6 KaiGai Kohei kai...@ak.jp.nec.com: The attached patch is an update of largeobject access controls. it applies fine (just 3 succeded hunks), compiles and passes regression tests... ALTER LARGE OBJECT is working, but now that we can change the owner of a LO we should be able to see who the actual owner is... i mean we should add an owner column in \dl for psql (maybe \dl+) and maybe an lo_owner() function. the GRANTs (and REVOKEs) work just fine too... Another question is what we want that only the LO owner can delete it (via lo_unlink)? another one, is it possible for us to have a CREATE LARGE OBJECT statement? the reason for this is: 1) it is a little ugly to use the OID in ALTER/GRANT/REVOKE statements, in a create statement we can assign a name to the LO 2) it could be more consistent with other ALTER/GRANT/REVOKE that acts over objects created with CREATE while large objects are created via lo_import() which makes obvious that are just records in meta-data table (hope this is understandable) It adds a new guc variable to turn on/off compatible behavior in largeobejct access controls. largeobject_compat_dac = [on | off] (default: off) If the variable is turned on, all the new access control features on largeobjects are bypassed, as if v8.4.x or prior did. the GUC works as intended but i don't like the name, it is not very meaningful for those of us that doesn't know what DAC or MAC are... another point, you really have to put the GUC in the postgresql.conf file if you hope people know about it ;) it is not documented either About the code... - I don't like the name pg_largeobject_meta why not pg_largeobject_acl (put here any other name you like)? or there was a reason for that choose? -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] numeric_to_number() function skipping some digits
Hi,
On Sat, Sep 19, 2009 at 1:52 AM, Brendan Jurd dire...@gmail.com wrote:
2009/9/19 Tom Lane t...@sss.pgh.pa.us:
Should we have it throw an error if the input corresponding to a G
symbol doesn't match the expected group separator? I'm concerned that
that would break applications that work okay today.
It would be a substantial change to the behaviour, and to do it
properly we'd have to change to_date() to actually parse separator
characters as well.
That is, you can currently write to_date('2009/09/19', '-MM-DD')
-- it doesn't matter what the separator characters actually look like,
since per the format pattern they cannot affect the date outcome.
This naturally leads to the question we always have to ask with these
functions: What Does Oracle Do?
Oracle returns 19-SEP-09 irrespective of the format.
Here in PG, we have getting the proper date irrespective of the format as
Oracle. But in the case to to_number the returned value is wrong. For
example following query returns '340' on PG where as it returns '3450' on
Oracle.
select to_number('34,50','999,99') from dual;
But FWIW, a -1 from me for changing this.
Do you mean this is the expected behaviour then?
Cheers,
BJ
--
Jeevan B Chalke
EnterpriseDB Software India Private Limited, Pune
Visit us at: www.enterprisedb.com
---
If better is possible, then good is not enough
Re: [HACKERS] numeric_to_number() function skipping some digits
2009/9/21 Jeevan Chalke jeevan.cha...@enterprisedb.com:
Oracle returns 19-SEP-09 irrespective of the format.
Here in PG, we have getting the proper date irrespective of the format as
Oracle. But in the case to to_number the returned value is wrong. For
example following query returns '340' on PG where as it returns '3450' on
Oracle.
select to_number('34,50','999,99') from dual;
Hi Jeevan,
Thanks for checking up on the Oracle behaviour. It appears to
silently disregard grouping characters in the format pattern, and also
disregard them wherever they appear in the input string (or else it
reads the string from right-to-left?).
It seems that, to match Oracle, we'd need to teach the code that 'G'
and ',' are no-ops for to_number(), and also that such characters
should be ignored in the input.
To be honest, though, I'm not sure it's worth pursuing. If you want
to feed in numbers that have decorative characters all through them,
it's far more predictable to just regex out the cruft and use ordinary
numeric parsing than to use to_number(), which is infamous for its
idiosyncrasies:
# SELECT regexp_replace('34,50', E'[\\d.]', '', 'g')::numeric;
3450
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] numeric_to_number() function skipping some digits
2009/9/21 Brendan Jurd dire...@gmail.com:
# SELECT regexp_replace('34,50', E'[\\d.]', '', 'g')::numeric;
3450
Sorry, that regex ought to have read E'[^\\d.]'.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Linux LSB init script
On Sun, 2009-09-20 at 22:54 -0400, Robert Haas wrote: It seems like there is some support for what this patch is trying to do, but much disagreement about the details of how to get there. Where do we go from here? I think the next step would be to outline what changes would be necessary in pg_ctl to implement LSB behavior. And then decide case by case whether it should become the default, an option, or is not appropriate for pg_ctl. Kevin apparently sort of agreed to do that when he came back. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Streaming Replication patch for CommitFest 2009-09
Having gone through the patch now in more detail, I think it's in pretty good shape. I'm happy with the overall design, except that I haven't been able to make up my mind if walreceiver should indeed be a stand-alone program as discussed, or a postmaster child process as in the patch you submitted. Putting that question aside for a moment, here's some minor things, in no particular order: - The async API in PQgetXLogData is quite different from the other commands. It's close to the API from PQgetCopyData(), but doesn't return a malloc'd buffer like PQgetCopyData does. I presume that's to optimize away the extra memcpy step? I don't think that's really necessary, I don't recall any complaints about that in PQgetCopyData(), and if it does become an issue, it could be optimized away by mallocing the buffer first and reading directly to that. - Can we avoid sprinkling XLogStreamingAllowed() calls to places where we check if WAL-logging is required (nbtsort.c, copy.c etc.). I think we need a new macro to encapsulate (XLogArchivingActive() || XLogStreamingAllowed()). - Is O_DIRECT ever a good idea in walreceiver? If it's really direct and doesn't get cached, the startup process will need to read from disk. - Can we replace read/write_conninfo with just a long-enough field in shared mem? Would be simpler. (this is moot if we go with the stand-alone walreceiver program and pass it as a command-line argument) - walreceiver shouldn't die on connection error, just to be restarted by startup process. Can we add error handling a la bgwriter and have a retry loop within walreceiver? (again, if we go with a stand-alone walreceiver program, it's probably better to have startup process responsible to restart walreceiver, as it is now) - pq_wait in backend waits until you can read or write at least 1 byte. There is no guarantee that you can send or read the whole message without blocking. We'd have to put the socket in non-blocking mode for that. I'm not sure what the implications of this are. - we should include system_identifier somewhere in the replication startup handshake. Otherwise you can connect to server from a different system and have logs shipped, if they happen to be roughly at the same point in WAL. Replay will almost certainly fail, but we should error earlier. - I know I said we should have just asynchronous replication at first, but looking ahead, how would you do synchronous? What kind of signaling is needed between walreceiver and startup process for that? - 'replication' shouldn't be a real database. I found the paging logic in walsender confusing, and didn't like the idea that walsender needs to set the XLOGSTREAM_END_SEG flag. Surely walreceiver knows how to split the WAL into files without such a flag. I reworked that logic, I think it's easier to understand now. I kept the support for the flag in libpq and the protocol for now, but it should be removed too, or repurposed to indicate that pg_switch_xlog() was done in the master. I've pushed that to 'replication-orig' branch in my git repository, attached is the same as a diff against your SR_0914.patch. I need a break from this patch, so I'll take a closer look at Simon's hot standby now. Meanwhile, can you work on the above items and submit a new version, please? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com *** a/src/backend/access/transam/recovery.conf.sample --- b/src/backend/access/transam/recovery.conf.sample *** *** 2,10 # PostgreSQL recovery config file # --- # ! # Edit this file to provide the parameters that PostgreSQL ! # needs to perform an archive recovery of a database, or ! # a log-streaming replication. # # If recovery.conf is present in the PostgreSQL data directory, it is # read on postmaster startup. After successful recovery, it is renamed --- 2,10 # PostgreSQL recovery config file # --- # ! # Edit this file to provide the parameters that PostgreSQL needs to ! # perform an archive recovery of a database, or to act as a log-streaming ! # replication standby. # # If recovery.conf is present in the PostgreSQL data directory, it is # read on postmaster startup. After successful recovery, it is renamed *** *** 83,89 #--- # # When standby_mode is enabled, the PostgreSQL server will work as ! # the standby. It tries to connect to the primary according to the # connection settings primary_conninfo, and receives XLOG records # continuously. # --- 83,89 #--- # # When standby_mode is enabled, the PostgreSQL server will work as ! # a standby. It tries to connect to the primary according to the # connection settings primary_conninfo, and receives XLOG records # continuously. # *** a/src/backend/access/transam/xlog.c
Re: [HACKERS] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
Jeff Janes írta: On Thu, Sep 3, 2009 at 6:47 AM, Boszormenyi Zoltan z...@cybertec.at mailto:z...@cybertec.at wrote: Boszormenyi Zoltan írta: Okay, we implemented only the lock_timeout GUC. Patch attached, hopefully in an acceptable form. Documentation included in the patch, lock_timeout works the same way as statement_timeout, takes value in milliseconds and 0 disables the timeout. Best regards, Zoltán Böszörményi New patch attached. It's only regenerated for current CVS so it should apply cleanly. I'm getting segfaults, built in 32 bit linux with gcc bin/pg_ctl -D data start -l logfile -o --lock_timeout=5 Session 1: jjanes=# begin; BEGIN jjanes=# select * from pgbench_branches where bid=3 for update; bid | bbalance | filler -+--+ 3 | -3108950 | (1 row) Session 2: jjanes=# select * from pgbench_branches where bid=3 for update; ERROR: could not obtain lock on row in relation pgbench_branches jjanes=# select * from pgbench_branches where bid=3 for update; ERROR: could not obtain lock on row in relation pgbench_branches jjanes=# select * from pgbench_branches where bid=3 for update; ERROR: could not obtain lock on row in relation pgbench_branches jjanes=# set lock_timeout = 0 ; SET jjanes=# select * from pgbench_branches where bid=3 for update; Session 2 is now blocked Session1: jjanes=# commit; long pause server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. I just realized I should have built with asserts turned on. I'll do that tomorrow, but don't want to delay this info until then, so I am sending it now. Cheers, Jeff Thanks for the test. The same test worked perfectly at the time I posted it and it also works perfectly on 8.4.1 *now*. So something has changed between then and the current CVS, because I was able to reproduce the segfault with the current CVS HEAD. We'll have to update the patch obviously... Best regards, Zoltán Böszörményi -- Bible has answers for everything. Proof: But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil. (Matthew 5:37) - basics of digital technology. May your kingdom come - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
Jeff Janes írta: On Thu, Sep 3, 2009 at 6:47 AM, Boszormenyi Zoltan z...@cybertec.at mailto:z...@cybertec.at wrote: Boszormenyi Zoltan írta: Alvaro Herrera írta: Boszormenyi Zoltan wrote: The vague consensus for syntax options was that the GUC 'lock_timeout' and WAIT [N] extension (wherever NOWAIT is allowed) both should be implemented. Behaviour would be that N seconds timeout should be applied to every lock that the statement would take. In http://archives.postgresql.org/message-id/291.1242053...@sss.pgh.pa.us Tom argues that lock_timeout should be sufficient. I'm not sure what does WAIT [N] buy I disagree with Tom on this point. *If* I was trying to implement a server policy, then sure, it should not be done by embedding the timeout in the SQL statement. But I don't think they want this to implement a server policy. (And if we do, why would we thump the poor victims that are waiting on the lock, rather than the rogue who decided to take a lock and then camp out on it?) The use case for WAIT [N] is not a server policy, but a UI policy. I have two ways to do this task. The preferred way needs to lock a row, but waiting for it may take too long. So if I can't get the lock within a reasonable time, I fall back on a less-preferred but still acceptable way of doing the task, one that doesn't need the lock. If we move to a new server, the appropriate value for the time out does not change, because the appropriate level is the concern of the UI and the end users, not the database server. This wouldn't be scattered all over the application, either. In my experience, if you have an application that could benefit from this, you might have 1 or 2 uses for WAIT [N] out of 1,000+ statements in the application. (From my perspective, if there were to be a WAIT [N] option, it could plug into the statement_timeout mechanism rather than the proposed lock_timeout mechanism.) I think that if the use case for a GUC is to set it, run a single very specific statement, and then unset it, that is pretty clear evidence that this should not be a GUC in the first place. Maybe I am biased in this because I am primarily thinking about how I would use such a feature, rather than how Hans-Juergen intends to use it, and maybe those uses differ. Hans-Juergen, could you describe your use case a little bit more? Who do is going to be getting these time-out errors, the queries run by the web-app, or longer running back-office queries? And when they do get an error, what will they do about it? Our use case is to port a huge set of Informix apps, that use SET LOCK MODE TO WAIT N; Apparently Tom Lane was on the opinion that PostgreSQL won't need anything more in that regard. In case the app gets an error, the query (transaction) can be retried, the when can be user controlled. I tried to argue on the SELECT ... WAIT N part as well, but for our purposes currently the GUC is enough. Okay, we implemented only the lock_timeout GUC. Patch attached, hopefully in an acceptable form. Documentation included in the patch, lock_timeout works the same way as statement_timeout, takes value in milliseconds and 0 disables the timeout. Best regards, Zoltán Böszörményi New patch attached. It's only regenerated for current CVS so it should apply cleanly. In addition to the previously mentioned seg-fault issues when attempting to use this feature (confirmed in another machine, linux, 64 bit, and --enable-cassert does not offer any help), I have some more concerns about the patch. From the docs: doc/src/sgml/config.sgml Abort any statement that tries to lock any rows or tables and the lock has to wait more than the specified number of milliseconds, starting from the time the command arrives at the server from the client. If varnamelog_min_error_statement/ is set to literalERROR/ or lower, the statement that timed out will also be logged. A value of zero (the default) turns off the limitation. This suggests that all row locks will have this behavior. However, my experiments show that row locks attempted to be taken for ordinary UPDATE commands do not time out. If this is only intended to apply to SELECT FOR UPDATE, that should be documented here. It is documented elsewhere that this applies to SELECT...FOR UPDATE, but it is not documented that this the only row-locks it applies to. from the time the command arrives at the server. I am pretty sure this is not the desired behavior, otherwise how does it differ from statement_timeout? I think it must be a copy and paste error for the doc. For the implementation, I think the patch touches too much code. In particular, lwlock.c. Is the time spent waiting on ProcArrayLock significant
Re: [HACKERS] GRANT ON ALL IN schema
Abhijit Menon-Sen wrote:
I have not yet been able to do a complete review of this patch, but I am
posting this because I'll be travelling for a week starting tomorrow. My
comments are based mostly on reading the patch, and not on any intensive
testing of the feature. I have left the patch status unchanged at needs
review, although I think it's close to ready for committer.
Thanks for your review.
1. The patch did apply to HEAD and build cleanly, but there are now a
couple of minor (documentation) conflicts. (Sorry, I would have fixed
them and reposted a patch, but I'm running out of time right now.)
I fixed those conflicts in attached patch.
*** a/doc/src/sgml/ref/grant.sgml
--- b/doc/src/sgml/ref/grant.sgml
[...]
para
+There is also the possibility of granting permissions to all objects of
+given type inside one or multiple schemas. This functionality is supported
+for tables, views, sequences and functions and can done by using
+ALL {TABLES|SEQUENCES|FUNCTIONS} IN SCHEMA schemaname syntax in place
+of object name.
+ /para
+
+ para
2. Here I suggest the following wording:
para
You can also grant permissions on all tables, sequences, or
functions that currently exist within a given schema by specifying
ALL {TABLES|SEQUENCES|FUNCTIONS} IN SCHEMA schemaname in place of
an object name.
/para
3. I believe MySQL's grant all privileges on foo.* to someone grants
privileges on all existing objects in foo _but also_ on any objects
that may be created later. This patch only gives you a way to grant
privileges only on the objects currently within a schema. I strongly
prefer this behaviour myself, but I do think the documentation needs
a brief mention of this fact, to avoid surprising people. That's why
I added that currently exist to (2), above. Maybe another sentence
that specifically says that objects created later are unaffected is
in order. I'm not sure.
I'll leave the exact wording to commiter, but in the attached patch I
changed it to say all existing objects instead of all objects.
Except for above two changes and the fact that it's against current
head, the patch is exactly the same.
Thanks again.
--
Regards
Petr Jelinek (PJMODOS)
grantonall-2009-09-21.diff.gz
Description: Unix tar archive
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby 0.2.1
Simon Riggs wrote: OK, here is the latest version of the Hot Standby patchset. This is about version 30+ by now, but we should regard this as 0.2.1 Patch against CVS HEAD (now): clean apply, compile, no known bugs. Thanks! Attached is some minor comment and fixes, and some dead code removal. Also available in my git repository, branch 'hs-riggs'. The documentation talks about setting and checking default_transaction_read_only, but I think it doesn't say anything about transaction_read_only, which I find odd. This in particular: Users will be able to tell whether their session is read-only by + issuing SHOW default_transaction_read_only seems misleading, as you might have default_transaction_read_only=on, but still be able to do SET transaction_read_only, so the *session* isn't necessarily read-only. The only bug I've found is this that we seem to be missing conflict resolution for GiST index tuples deleted by the kill_prior_tuples mechanism. Unless I'm missing something, we need similar handling there that we have in b-tree. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index 91917cf..2257ec6 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -2099,9 +2099,9 @@ if (!triggered) para In recovery, transactions will not be permitted to take any lock higher - other than AccessShareLock or AccessExclusiveLock. In addition, - transactions may never assign a TransactionId and may never write WAL. - The LOCK TABLE command by default applies an AccessExclusiveLock. + than AccessShareLock. In addition, transactions may never assign a +TransactionId and may never write WAL. + The LOCK TABLE command by default applies an AccessExclusiveLock. Any LOCK TABLE command that runs on the standby and requests a specific lock type other than AccessShareLock will be rejected. /para @@ -2168,8 +2168,8 @@ if (!triggered) para An example of the above would be an Administrator on Primary server - runs a DROP TABLE command that refers to a table currently in use by - a User query on the standby server. + runs a DROP TABLE command on a table that's currently being queried + in the standby server. /para para @@ -2198,9 +2198,9 @@ if (!triggered) para We have a number of choices for resolving query conflicts. The default is that we wait and hope the query completes. If the recovery is not paused, - then the server will wait automatically until the server the lag between + then the server will wait automatically until the lag between primary and standby is at most max_standby_delay seconds. Once that grace - period expires, we then take one of the following actions: + period expires, we take one of the following actions: itemizedlist listitem @@ -2213,7 +2213,7 @@ if (!triggered) para If the conflict is caused by cleanup records we tell the standby query that a conflict has occurred and that it must cancel itself to avoid the - risk that it attempts to silently fails to read relevant data because + risk that it silently fails to read relevant data because that data has been removed. (This is very similar to the much feared error message snapshot too old). /para @@ -,7 +,7 @@ if (!triggered) Note also that this means that idle-in-transaction sessions are never canceled except by locks. Users should be clear that tables that are regularly and heavily updated on primary server will quickly cause - cancellation of any longer running queries made against those tables. + cancellation of any longer running queries in the standby. /para para @@ -2235,7 +2235,7 @@ if (!triggered) /para para - Other remdial actions exist if the number of cancelations is unacceptable. + Other remedial actions exist if the number of cancelations is unacceptable. The first option is to connect to primary server and keep a query active for as long as we need to run queries on the standby. This guarantees that a WAL cleanup record is never generated and we don't ever get query @@ -2283,7 +2283,7 @@ if (!triggered) titleAdministrator's Overview/title para - If there is a recovery.conf file present then the will start in Hot Standby + If there is a recovery.conf file present the server will start in Hot Standby mode by default, though this can be disabled by setting recovery_connections = off in recovery.conf. The server may take some time to enable recovery connections since the server must first complete @@ -2329,7 +2329,7 @@ LOG: database system is ready to accept read only connections A set of functions allow superusers to control the flow of recovery are described in xref linkend=functions-recovery-control-table. These functions allow you to pause and continue recovery, as well - as dynamically set new recovery targets wile recovery progresses. + as dynamically set new
Re: [HACKERS] [PATCH] DefaultACLs
Hi,
here's a (late, sorry about that) review:
== Trivia ==
Patch applies cleanly with a few 1 line offsets.
It's unified, not context, but that's trivial.
The patch adds some trailing whitespace, which is not good (git diff
shows it in red, it's easy to spot it). There's also one
hunk that's just an addition of a newline (in
src/backend/catalog/aclchk.c, -270,6 +291,7)
== Code ==
There's a few places where the following pattern is used:
if (!stmt-grantees)
whereas I think the project prefers:
stmt-grantees != NIL
Same for if (schemas) = if (schemas != NULL)
I'm not sure if this pattern in src/backend/catalog/aclchk.c is the best
option:
if (rolenames == NIL)
rolenames = lappend(rolenames, makeString(pstrdup()));
if (nspnames == NIL)
nspnames = lappend(nspnames, makeString(pstrdup()));
Appending an empty string and then checking in strlen of the option
value is 0
is ugly.
In SetDefaultACLs the OidIsValid(roleId) is not necessary, maybe better
put in
assert(oidIsValid). The caller always puts a valid rolename in there. Or
maybe
even better: make the caller pass an InvalidOid for the role if no is
specified. This way the handling arguments is more uniform between
SetDefaultACLs and ExecDefaultACLsStmt.
The logic in get_default_acl and pg_namespace_object_default_acl could be
improved, for instance get_default_acl checks if the result of the scan
is null
and if is, returns NULL. At the same time, the only calling function,
pg_namespace_object_default_acl checks the isNull flag instead of just
checking
if the result is NULL.
Also, pg_namespace_object_default_acl could just do without the isNull out
parameter and the same goes for get_default_acl. Just return NULL to
indicate
an invalid result and declare a isNull in get_default_acl locally to use
it in
heap_getattr. This also saves some lines in InsertPgClassTuple.
Also, in InsertPgClassTuple change the order of the branches:
+ if (isNull)
+ nulls[Anum_pg_class_relacl - 1] = true;
+ else
+ values[Anum_pg_class_relacl - 1] = PointerGetDatum(relacl);
to have the same pattern as the next if statement.
Also, changing tests like if (!isNull) to if (relacl) makes it more
natural to
see sequences like if (relacl) { do-stuff; pfree(relacl); }
In ExecDefaultACLsStmt this fragment:
else if (strcmp(defel-defname, roles) == 0)
{
if (rolenames)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg(conflicting or redundant options)));
drolenames = defel;
}
Should test if (drolenames), because currently it's possible to do:
alter default privileges for role test for role test grant select, insert on
table to test;
Maybe add a unit test for that.
A comment in dependency.c function getDefaultACLDescription with
something like
shouldn't get here in the default: switch branch could be useful,
cf. getRelationDescription.
In ExecGrantDefaults_Relation there's a hunk:
+ if (isNull)
+ elog(ERROR, no DEFAULT PRIVILEGES for relation);
Maybe you could move it higher, no need to do other stuff if it's going
to fail
afterwards anyway.
ExecGrantDefaults_Function and ExecGrantDefaults_Relation could maybe share
code? They look quite similar, although it might not be so easy to
factor out
common functionality.
The unrecognized GrantStmt.objtype: %d error message needs better
wording I
think.
No code patch removes rows from pg_default_acls, so it might accumulate
cruft. Maybe a drop default privileges? Or maybe revoking all would delete
the row instead of setting it? It has the same meaning, I guess...
== Compiling and installing ==
My gcc complains about
gram.y: In function ‘base_yyparse’:
gram.y:1128: warning: assignment from incompatible pointer type
gram.y:1135: warning: passing argument 1 of ‘lappend’ from incompatible
pointer type
gram.y:1135: warning: assignment from incompatible pointer type
gram.y:1136: warning: assignment from incompatible pointer type
Regression tests fail because of the username mismatch
! DETAIL: drop cascades to default acls for role postgres on new
relation in namespace regressns
--- 951,957
! DETAIL: drop cascades to default acls for role wulczer on new
relation in namespace regressns
== Testing ==
Tab completion is not up to speed - annoying ;)
The functionality worked as advertised, although I was able to do the
following:
postgres=# create role test login;
CREATE ROLE
postgres=# \c - test
psql (8.5devel)
You are now connected to database postgres as user test.
postgres= alter default privileges grant select on table to test;
ALTER DEFAULT PRIVILEGES
postgres= \c - wulczer
psql (8.5devel)
You are now connected to database postgres as user wulczer.
postgres=# drop role test;
DROP ROLE
postgres=# select * from pg_default_acls ;
defaclrole | defaclnamespace | defaclobjtype | defacllist
+-+---+---
16384 | 0 | r | {16384=arwdDxt/16384}
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
--On 20. September 2009 22:56:53 -0400 Robert Haas robertmh...@gmail.com wrote: So is this ready to commit, or what? Not yet, see the comments Alvaro did upthread. Please note that i'm still reviewing this one and i hope to post results tomorrow (there wasn't plenty of free time over the weekend, i'm sorry). -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby 0.2.1
On Mon, 2009-09-21 at 13:50 +0300, Heikki Linnakangas wrote: The only bug I've found ! is this that we seem to be missing conflict resolution for GiST index tuples deleted by the kill_prior_tuples mechanism. Unless I'm missing something, we need similar handling there that we have in b-tree. OK, I agree with that. Straightforward change. Thanks very much. I marked the comment to indicate that the handling for GIST and GIN indexes looked dubious to me also. I had the earlier it is safe comments but that was before we looked at the kill prior tuples issue. Re-reading code for GIN also, I note that there isn't any further work because we don't kill prior tuples ever. Also, there is no special handling of the GIN b-tree posting tree because VACUUM scans that in logical sequence, rather than the physical sequence in nbtree. -- Simon Riggs www.2ndQuadrant.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] Hot Standby 0.2.1
On Mon, 2009-09-21 at 13:50 +0300, Heikki Linnakangas wrote: The documentation talks about setting and checking default_transaction_read_only, but I think it doesn't say anything about transaction_read_only, which I find odd. This in particular: Users will be able to tell whether their session is read-only by + issuing SHOW default_transaction_read_only seems misleading, as you might have default_transaction_read_only=on, but still be able to do SET transaction_read_only, so the *session* isn't necessarily read-only. Yes, clearly missing a check there. Those two operations should be blocked at higher level, using PreventCommandDuringRecovery() and I confess that I thought they already were. Doesn't crash because of the other checks in place, but gives wrong error message. Thanks for penetration testing the patch. -- Simon Riggs www.2ndQuadrant.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] WIP: generalized index constraints
On Sun, 2009-09-20 at 10:08 -0700, Jeff Davis wrote: On Sun, 2009-09-20 at 13:01 -0400, Tom Lane wrote: The current infrastructure for deferred uniqueness requires that the thing actually be a constraint, with an entry in pg_constraint that can carry the deferrability options. So unless we want to rethink that, this might be a sufficient reason to override my arguments about not wanting to use CONSTRAINT syntax. Ok. Using the word EXCLUSION would hopefully guard us against future changes to SQL, but you know more about the subtle dangers of language changes than I do. So, do I still omit it from information_schema? I would say yes. Overall, I think this terminology is pretty good now. We could say, PostgreSQL has a new constraint type, exclusion constraint. It shares common properties with other constraint types, e.g., deferrability (in the future), ADD/DROP CONSTRAINT, etc. But because the standard does not describe exclusion constraints, they are not listed in the information schema. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Resjunk sort columns, Heikki's index-only quals patch, and bug #5000
Robert Haas wrote: Since you previously stated that you were going to put this patch aside to work on HS and SR[1], I'm going to move this to Returned with Feedback for now. Hope that's OK, and that the feedback is sufficient and useful. Yes, on both counts. Thank you! -- 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] TODO item: Allow more complex user/database default GUC settings
On Mon, Sep 21, 2009 at 12:21 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Robert Haas escribió: Here's a first shot on this for my current review round. Patch needed to re-merged into current CVS HEAD due to some merge conflicts, i've also adjusted the regression tests (minor). On a first look, i like the patch (especially the code for the utility commands accessing the settings is better modularized now, which looks much nicer). So is this ready to commit, or what? Not really :-( It needs some minor tweaks to qualify as a cleanup patch, and further extra coding for there to be an actual new feature. So here's the followup question - do you intend to do one of those things for this CommitFest, or should we mark this as Returned with Feedback once Bernd posts the rest of his review? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic copy options
Emmanuel Cecchet m...@frogthinker.org writes: The easiest for both implementation and documentation might just be to have a matrix of options. Each option has a row and a column in the matrix. The intersection of a row and a column is set to 0 if options are not compatible and set to 1 if it is. This way we are sure to capture all possible combinations. This way, each time we find a new option, we just have to check in the matrix if it is compatible with the already existing options. Note that we can also replace the 0 with an index in an error message array. This seems like overkill at the moment. Maybe when/if we get to actually supporting three or more COPY formats, we'd need it. Right now all we are trying to do is make the grammar not be a factor in adding options, and the foreseen new options aren't about new formats at all. So I'm inclined to just fix the grammar and not do any refactoring of the code in copy.c. As far as I can tell, the majority opinion is to use format csv and not have the csv_ prefixes on the options, so I will adjust the patch accordingly and commit it (barring any other problems coming up when I read it more closely). 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] TODO item: Allow more complex user/database default GUC settings
Robert Haas escribió: So here's the followup question - do you intend to do one of those things for this CommitFest, or should we mark this as Returned with Feedback once Bernd posts the rest of his review? What feedback is it supposed to be returned with? Precisely what I wanted is some feedback on the general idea. Brendan's I like the approach is good, but is it enough to deter a later veto from someone else? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera escribió: What feedback is it supposed to be returned with? Precisely what I wanted is some feedback on the general idea. Brendan's I like the approach is good, but is it enough to deter a later veto from someone else? s/Brendan/Bernd/ -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
I think that if the use case for a GUC is to set it, run a single very specific statement, and then unset it, that is pretty clear evidence that this should not be a GUC in the first place. +1 Plus, do we really want another GUC? -- Josh Berkus PostgreSQL Experts Inc. www.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] updated hstore patch
On Sep 20, 2009, at 3:14 PM, Andrew Gierth wrote: I think you're missing the point here; I can't control what it resolves to, since that's the job of the function overload resolution code. Yeah, but I think that the existing behavior is probably the best. But I checked, and delete(hstore,$1) still resolves to delete(hstore,text) when the type of $1 is not specified, so there's no compatibility issue there that I can see. (I'm not sure I understand _why_ it resolves to that rather than being ambiguous...) Right, but it does seem like it might be the best choice for now. I'd add a regression test to make sure it stays that way. David So then it's negligible for new values? Yes. (One bit test, done inline) Excellent, thanks. David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] updated hstore patch
On Sep 20, 2009, at 12:15 PM, Tom Lane wrote: That recipe doesn't actually work for cases like this. What *would* work is loading the module *before* restoring from your old dump, then relying on the CREATEs from the incoming dump to fail. Jesus this is hacky, either way. :-( I believe we have already discussed the necessity for pg_upgrade to support this type of subterfuge. A module facility would be a lot better of course, but we still need something for upgrading existing databases that don't contain the module structure. Yeah, it's past time for a real module facility. 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
Re: [HACKERS] generic copy options
I wrote: As far as I can tell, the majority opinion is to use format csv BTW, if we're going to do that, shouldn't the binary option instead be spelled format binary? 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] generic copy options
Tom Lane wrote: I wrote: As far as I can tell, the majority opinion is to use format csv BTW, if we're going to do that, shouldn't the binary option instead be spelled format binary? Looking at the doc, it looks like FORMAT should be mandatory and be either text, binary or csv (for now). manu -- Emmanuel Cecchet Aster Data Systems Web: http://www.asterdata.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] generic copy options
On Mon, Sep 21, 2009 at 1:51 PM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: As far as I can tell, the majority opinion is to use format csv BTW, if we're going to do that, shouldn't the binary option instead be spelled format binary? Good catch, +1. ...Robert -- Sent 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_hba.conf: samehost and samenet [REVIEW]
Thanks for your review!
Abhijit Menon-Sen wrote:
First, it needs to be reformatted to not use a space before the opening
parentheses in (some) function calls and definitions.
Fixed in the attached patch.
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
[...]
I'd suggest something like the following instead:
paraInstead of a replaceableCIDR-address/replaceable, you can
specify literalsamehost/literal to match any of the server's own
IP addresses, or literalsamenet/literal to match any address in
a subnet that the server belongs to.
Updated in attached patch.
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
[...]
+else if (addr-sa_family == AF_INET
+ raddr-addr.ss_family == AF_INET6)
+{
+/*
+ * Wrong address family. We allow only one case: if the file
+ * has IPv4 and the port is IPv6, promote the file address to
+ * IPv6 and try to match that way.
+ */
How about this instead:
If we're listening on IPv6 but the file specifies an IPv4 address to
match against, we promote the latter also to an IPv6 address before
trying to match the client's address.
As Magnus noted, this is a comment already present in the postgresql
code. I simply moved it into a function. However, I've attached a second
patch which fixes this issue, and can be committed at your discretion.
You could just have each of the three #ifdef blocks
define a function named pg_foreach_ifaddr() and be done with it. No
need for a fourth function.
Done.
*** a/src/backend/libpq/pg_hba.conf.sample
--- b/src/backend/libpq/pg_hba.conf.sample
[...]
+ # You can also specify samehost to limit connections to those from
addresses
+ # of the local machine. Or you can specify samenet to limit connections
+ # to addresses on the subnets of the local network.
This should be reworded to match the documentation change suggested
above.
Done.
Cheers,
Stef
diff --git a/configure.in b/configure.in
index e545a1f..b77ce2b 100644
*** a/configure.in
--- b/configure.in
*** AC_SUBST(OSSP_UUID_LIBS)
*** 969,975
##
dnl sys/socket.h is required by AC_FUNC_ACCEPT_ARGTYPES
! AC_CHECK_HEADERS([crypt.h dld.h fp_class.h getopt.h ieeefp.h langinfo.h poll.h pwd.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/socket.h sys/shm.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h kernel/OS.h kernel/image.h SupportDefs.h])
# At least on IRIX, cpp test for netinet/tcp.h will fail unless
# netinet/in.h is included first.
--- 969,975
##
dnl sys/socket.h is required by AC_FUNC_ACCEPT_ARGTYPES
! AC_CHECK_HEADERS([crypt.h dld.h fp_class.h getopt.h ieeefp.h langinfo.h poll.h pwd.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/socket.h sys/shm.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h kernel/OS.h kernel/image.h SupportDefs.h ifaddrs.h])
# At least on IRIX, cpp test for netinet/tcp.h will fail unless
# netinet/in.h is included first.
*** PGAC_VAR_INT_TIMEZONE
*** 1148,1154
AC_FUNC_ACCEPT_ARGTYPES
PGAC_FUNC_GETTIMEOFDAY_1ARG
! AC_CHECK_FUNCS([cbrt dlopen fcvt fdatasync getpeereid getpeerucred getrlimit memmove poll pstat readlink setproctitle setsid sigprocmask symlink sysconf towlower utime utimes waitpid wcstombs])
# posix_fadvise() is a no-op on Solaris, so don't incur function overhead
# by calling it, 2009-04-02
--- 1148,1154
AC_FUNC_ACCEPT_ARGTYPES
PGAC_FUNC_GETTIMEOFDAY_1ARG
! AC_CHECK_FUNCS([cbrt dlopen fcvt fdatasync getpeereid getpeerucred getrlimit memmove poll pstat readlink setproctitle setsid sigprocmask symlink sysconf towlower utime utimes waitpid wcstombs getifaddrs])
# posix_fadvise() is a no-op on Solaris, so don't incur function overhead
# by calling it, 2009-04-02
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index ad4d084..e5152f4 100644
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
*** hostnossl replaceabledatabase/replac
*** 244,249
--- 244,255
support for IPv6 addresses.
/para
+ paraInstead of a replaceableCIDR-address/replaceable, you can specify
+literalsamehost/literal to match any of the server's own IP addresses,
+or literalsamenet/literal to match any address in a subnet that the
+server belongs to.
+ /para
+
para
This field only applies to literalhost/literal,
literalhostssl/literal, and literalhostnossl/ records.
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index e6f7db2..702971a 100644
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
*** check_db(const char *dbname, const char
*** 512,517
--- 512,608
return false;
}
+ /*
+ *
Re: [HACKERS] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
On Mon, Sep 21, 2009 at 1:32 PM, Josh Berkus j...@agliodbs.com wrote: I think that if the use case for a GUC is to set it, run a single very specific statement, and then unset it, that is pretty clear evidence that this should not be a GUC in the first place. +1 Plus, do we really want another GUC? Well, I don't share the seemingly-popular sentiment that more GUCs are a bad thing. GUCs let you change important parameters of the application without compiling, which is very useful. Of course, I don't want: - GUCs that I'm going to set, execute one statement, and the unset (and this likely falls into that category). - GUCs that are poorly designed so that it's not clear, even to an experienced user, what value to set. - GUCs that exist only to work around the inability of the database to figure out the appropriate value without user input. On the flip side, rereading the thread, one major advantage of the GUC is that it can be used for statements other than SELECT, which hard-coded syntax can't. That might be enough to make me change my vote. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Standalone backends run StartupXLOG in an incorrect environment
Alvaro Herrera alvhe...@commandprompt.com writes: So if you need to enter standalone mode, you'll have to start postmaster, wait for replay to finish, stop it and restart standalone. Yeah, that's the case at the moment. Would this be a problem when you need standalone mode in an emergency, for example when the database won't start due to Xid wraparound? If it won't run through StartupXLOG under the postmaster, it's not going to do so standalone either. 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] Adding \ev view editor?
Tom Lane wrote:
It might be worth pointing out that what I don't want pg_dump doing
is suppressing useless parentheses. Adding whitespace ought to be
safe enough. So if anyone wanted to do the work of decoupling those
two effects of the prettyprint option, we could have semi pretty
printed output in pg_dump. Dunno if it's worth it.
The attached patch goes part of the way towards doing this, by adding
white space unconditionally to the target list of a viewdef.
The nice thing about that is that it's the part of a viewdef which
basically isn't pretty-printed now, even if you ask for pretty printing.
That would certainly
make editing the view with a \ev command a lot nicer.
Along the way it suppresses putting a newline before a CASE expression
in pretty printing mode, which seems to be an odd thing to do and is
inconsistent with the way other expressions are treated.
Sample output:
andrew=# select pg_get_viewdef('foo',true);
pg_get_viewdef
--
SELECT 'a'::text AS b,
( SELECT 1
FROM dual) AS x,
random() AS y,
CASE
WHEN true THEN 1
ELSE 0
END AS c,
1 AS d
FROM dual;
(1 row)
cheers
andrew
Index: src/backend/utils/adt/ruleutils.c
===
RCS file: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v
retrieving revision 1.306
diff -c -r1.306 ruleutils.c
*** src/backend/utils/adt/ruleutils.c 1 Aug 2009 19:59:41 - 1.306
--- src/backend/utils/adt/ruleutils.c 21 Sep 2009 18:22:01 -
***
*** 2648,2659
TupleDesc resultDesc)
{
StringInfo buf = context-buf;
char *sep;
! int colno;
ListCell *l;
sep = ;
colno = 0;
foreach(l, targetList)
{
TargetEntry *tle = (TargetEntry *) lfirst(l);
--- 2648,2671
TupleDesc resultDesc)
{
StringInfo buf = context-buf;
+ StringInfoData sep_indent;
char *sep;
! int colno, len;
ListCell *l;
sep = ;
colno = 0;
+
+ /*
+ * Separator to make each target appear on its own line, regardless
+ * of pretty printing.
+ * Try to make them all line up at the same line position.
+ */
+ initStringInfo(sep_indent);
+ appendStringInfoString(sep_indent,,\n );
+ for (len = buf-len -1; len = 0 buf-data[len]!= '\n'; len--)
+ appendStringInfoChar(sep_indent,' ');
+
foreach(l, targetList)
{
TargetEntry *tle = (TargetEntry *) lfirst(l);
***
*** 2664,2670
continue; /* ignore junk entries */
appendStringInfoString(buf, sep);
! sep = , ;
colno++;
/*
--- 2676,2682
continue; /* ignore junk entries */
appendStringInfoString(buf, sep);
! sep = sep_indent.data;
colno++;
/*
***
*** 2704,2709
--- 2716,2726
appendStringInfo(buf, AS %s, quote_identifier(colname));
}
}
+
+ if (colno 1 ! PRETTY_INDENT(context))
+ appendStringInfoChar(buf,'\n');
+
+ pfree(sep_indent.data);
}
static void
***
*** 4595,4602
CaseExpr *caseexpr = (CaseExpr *) node;
ListCell *temp;
! appendContextKeyword(context, CASE,
! 0, PRETTYINDENT_VAR, 0);
if (caseexpr-arg)
{
appendStringInfoChar(buf, ' ');
--- 4612,4620
CaseExpr *caseexpr = (CaseExpr *) node;
ListCell *temp;
! appendStringInfoString(buf,CASE);
! if (PRETTY_INDENT(context))
! context-indentLevel += PRETTYINDENT_VAR;
if (caseexpr-arg)
{
appendStringInfoChar(buf, ' ');
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Standalone backends run StartupXLOG in an incorrect environment
Tom Lane wrote: Fixing this will require rearranging things around InitPostgres (in particular, I think InitBufferPoolBackend will have to be called directly from postgres.c). Since that code got rearranged quite a bit last month, I'd be hesitant to try to back-patch whatever fix we come up with for HEAD. Seeing that we'd never noticed the problem before, I think it's okay to fix it just in HEAD and not risk back-patching ... comments? So if you need to enter standalone mode, you'll have to start postmaster, wait for replay to finish, stop it and restart standalone. Would this be a problem when you need standalone mode in an emergency, for example when the database won't start due to Xid wraparound? Frankly I don't have a problem with no backpatching but ... -- Alvaro Herrerahttp://www.CommandPrompt.com/ 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] Adding \ev view editor?
Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: It might be worth pointing out that what I don't want pg_dump doing is suppressing useless parentheses. Adding whitespace ought to be safe enough. So if anyone wanted to do the work of decoupling those two effects of the prettyprint option, we could have semi pretty printed output in pg_dump. Dunno if it's worth it. The attached patch goes part of the way towards doing this, by adding white space unconditionally to the target list of a viewdef. That's not what I had in mind by decoupling the option's effects. 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
[HACKERS] Progress on Writeable CTEs
Hi, I've looked at implementing writeable CTEs on top of my DML node patch (repo here: git://git.postgresql.org/git/writeable_cte.git ) and encountered a few conundrums. You can see what I've done in the actually_write branch of that repo. - Currently we only store the OIDs of the result relations we're going to be operating on. The executor then decides whether to open the indices for the result relations or not based on the type of the top-level statement, but in the future we could have CTE subqueries operating on the result relations. Propagating result relation OIDs from the CTE subqueries leads to possibly having the same relations opened multiple times. Even if this isn't a problem, we don't know whether to open the indices or not. 1) If we want to have only a single ResultRelationInfo per result relation, eliminating the duplicates from a list would be pretty slow if we have a huge number of result relations. On the other hand, a hash (or similar) data structure would probably be an overkill. Updating a huge number of tables would probably already be painfully slow. Even if we didn't want to eliminate duplicate ResultRelationInfo structures, we currently don't know what operations we want to perform on which result relation, so: 2) we could unconditionally open indices for every result relation, or: 3) we could emit some info about what we're going to do along with the OID of the result relation. #1 would force some changes to parts of the code. For example, we'd need to move the RETURNING projection info from ResultRelationInfo to the DML node, but I was thinking of doing that even if we chose #2 or #3. There are also some other parts that would need to be touched, but that is the biggest issue I'm aware of. - The DML subqueries should go through rewrite. I've looked at this, and we could teach the rewrite subsystem to take a look at the queries of top-level CTEs and rewrite them if necessary. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
Robert Haas escribió: Of course, I don't want: - GUCs that I'm going to set, execute one statement, and the unset (and this likely falls into that category). - GUCs that are poorly designed so that it's not clear, even to an experienced user, what value to set. - GUCs that exist only to work around the inability of the database to figure out the appropriate value without user input. On the flip side, rereading the thread, one major advantage of the GUC is that it can be used for statements other than SELECT, which hard-coded syntax can't. That might be enough to make me change my vote. Perhaps we'd benefit from a way to set a variable for a single query; something like WITH ( SET query_lock_timeout = 5s ) SELECT ... Of course, this particular syntax doesn't work because WITH is already taken. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
On Mon, Sep 21, 2009 at 3:14 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Robert Haas escribió: Of course, I don't want: - GUCs that I'm going to set, execute one statement, and the unset (and this likely falls into that category). - GUCs that are poorly designed so that it's not clear, even to an experienced user, what value to set. - GUCs that exist only to work around the inability of the database to figure out the appropriate value without user input. On the flip side, rereading the thread, one major advantage of the GUC is that it can be used for statements other than SELECT, which hard-coded syntax can't. That might be enough to make me change my vote. Perhaps we'd benefit from a way to set a variable for a single query; something like WITH ( SET query_lock_timeout = 5s ) SELECT ... Of course, this particular syntax doesn't work because WITH is already taken. Yeah, I thought about that. I think that would be sweet. Maybe LET (query_lock_timeout = 5 s) IN SELECT ... ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Adding \ev view editor?
Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: It might be worth pointing out that what I don't want pg_dump doing is suppressing useless parentheses. Adding whitespace ought to be safe enough. So if anyone wanted to do the work of decoupling those two effects of the prettyprint option, we could have semi pretty printed output in pg_dump. Dunno if it's worth it. The attached patch goes part of the way towards doing this, by adding white space unconditionally to the target list of a viewdef. I'd rather do that than add another pg_get_viewdef variant or option. That's not what I had in mind by decoupling the option's effects. Well, regardless of that it does what I want, and with a fairly small amount of code. I can make it work only in the pretty print case, if that's your objection. Like you I doubt that fully decoupling pretty printing parentheses and whitespace is going to be worth the trouble. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
Alvaro Herrera alvhe...@commandprompt.com writes: Perhaps we'd benefit from a way to set a variable for a single query; Yeah, particularly if it allows us to fend off requests for random one-off features to accomplish the same thing ... WITH ( SET query_lock_timeout = 5s ) SELECT ... Of course, this particular syntax doesn't work because WITH is already taken. I think you could make it work if you really wanted, but perhaps a different keyword would be better. 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] Hot Standby 0.2.1
On Mon, 2009-09-21 at 14:01 +0100, Simon Riggs wrote: On Mon, 2009-09-21 at 13:50 +0300, Heikki Linnakangas wrote: is this that we seem to be missing conflict resolution for GiST index tuples deleted by the kill_prior_tuples mechanism. Unless I'm missing something, we need similar handling there that we have in b-tree. OK, I agree with that. Straightforward change. Thanks very much. I marked the comment to indicate that the handling for GIST and GIN indexes looked dubious to me also. I had the earlier it is safe comments but that was before we looked at the kill prior tuples issue. ISTM I looked at this too quickly. kill_prior_tuple is only ever set by these lines, after scan starts: if (!scan-xactStartedInRecovery) scan-kill_prior_tuple = scan-xs_hot_dead; which is set in indexam.c, not within any particular am. So the coding, as submitted, covers all index types, current and future. AFAICS there is no bug, unless you have a test case or can explain further? Worth raising as a query because it forced me to re-check how GIST and GIN work and am happy again now. -- Simon Riggs www.2ndQuadrant.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] Hot Standby 0.2.1
On Mon, Sep 21, 2009 at 9:01 AM, Simon Riggs si...@2ndquadrant.com wrote: On Mon, 2009-09-21 at 13:50 +0300, Heikki Linnakangas wrote: The only bug I've found ! Yeah, wow. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic copy options
Emmanuel Cecchet m...@asterdata.com writes: [ generic copy options patch ] Applied with revisions as discussed. 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] test_fsync file overrun
Jeff Janes wrote:
test_fsync in tools/fsync pre-creates a 16MB file. If it is given a number
of iterations greater than 1024 (like one might use if trying to see what
happens when NVRAM gets filled, or on a journaling file system), than one of
the writes being timed will have to extend the size of the pre-created test
file, which can greatly skew the results.
This patch uses lseek to periodically restart at the beginning of the file,
rather than writing past the end of it.
Oh, I never noticed that the later tests kept appending to the file
rather then overwriting it. I have applied the attached fix for CVS
HEAD that just uses lseek() before each write group, as you suggested.
I have backpatched it to 8.4.X because the original code created 16GB
files in tests (yikes).
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/tools/fsync/test_fsync.c
===
RCS file: /cvsroot/pgsql/src/tools/fsync/test_fsync.c,v
retrieving revision 1.24
diff -c -c -r1.24 test_fsync.c
*** src/tools/fsync/test_fsync.c 10 Aug 2009 18:19:06 - 1.24
--- src/tools/fsync/test_fsync.c 21 Sep 2009 16:52:00 -
***
*** 149,156
--- 149,160
die(Cannot open output file.);
gettimeofday(start_t, NULL);
for (i = 0; i loops; i++)
+ {
if (write(tmpfile, buf, WRITE_SIZE) != WRITE_SIZE)
die(write failed);
+ if (lseek(tmpfile, 0, SEEK_SET) == -1)
+ die(seek failed);
+ }
gettimeofday(elapse_t, NULL);
close(tmpfile);
printf(\tone 16k o_sync write );
***
*** 167,172
--- 171,178
die(write failed);
if (write(tmpfile, buf, WRITE_SIZE / 2) != WRITE_SIZE / 2)
die(write failed);
+ if (lseek(tmpfile, 0, SEEK_SET) == -1)
+ die(seek failed);
}
gettimeofday(elapse_t, NULL);
close(tmpfile);
***
*** 188,195
--- 194,205
die(Cannot open output file.);
gettimeofday(start_t, NULL);
for (i = 0; i loops; i++)
+ {
if (write(tmpfile, buf, WRITE_SIZE / 2) != WRITE_SIZE / 2)
die(write failed);
+ if (lseek(tmpfile, 0, SEEK_SET) == -1)
+ die(seek failed);
+ }
gettimeofday(elapse_t, NULL);
close(tmpfile);
printf(\topen o_dsync, write);
***
*** 205,212
--- 215,226
die(Cannot open output file.);
gettimeofday(start_t, NULL);
for (i = 0; i loops; i++)
+ {
if (write(tmpfile, buf, WRITE_SIZE / 2) != WRITE_SIZE / 2)
die(write failed);
+ if (lseek(tmpfile, 0, SEEK_SET) == -1)
+ die(seek failed);
+ }
gettimeofday(elapse_t, NULL);
close(tmpfile);
printf(\topen o_sync, write );
***
*** 226,231
--- 240,247
if (write(tmpfile, buf, WRITE_SIZE / 2) != WRITE_SIZE / 2)
die(write failed);
fdatasync(tmpfile);
+ if (lseek(tmpfile, 0, SEEK_SET) == -1)
+ die(seek failed);
}
gettimeofday(elapse_t, NULL);
close(tmpfile);
***
*** 246,251
--- 262,269
die(write failed);
if (fsync(tmpfile) != 0)
die(fsync failed);
+ if (lseek(tmpfile, 0, SEEK_SET) == -1)
+ die(seek failed);
}
gettimeofday(elapse_t, NULL);
close(tmpfile);
***
*** 269,274
--- 287,294
die(write failed);
if (write(tmpfile, buf, WRITE_SIZE / 2) != WRITE_SIZE / 2)
die(write failed);
+ if (lseek(tmpfile, 0, SEEK_SET) == -1)
+ die(seek failed);
}
gettimeofday(elapse_t, NULL);
close(tmpfile);
***
*** 290,295
--- 310,317
die(write failed);
if (write(tmpfile, buf, WRITE_SIZE / 2) != WRITE_SIZE / 2)
die(write failed);
+ if (lseek(tmpfile, 0, SEEK_SET) == -1)
+ die(seek failed);
}
gettimeofday(elapse_t, NULL);
close(tmpfile);
***
*** 310,315
--- 332,339
if (write(tmpfile, buf, WRITE_SIZE / 2) != WRITE_SIZE / 2)
die(write failed);
fdatasync(tmpfile);
+ if (lseek(tmpfile, 0, SEEK_SET) == -1)
+ die(seek failed);
}
gettimeofday(elapse_t, NULL);
close(tmpfile);
***
*** 332,337
--- 356,363
die(write failed);
if (fsync(tmpfile) != 0)
die(fsync failed);
+ if (lseek(tmpfile, 0, SEEK_SET) == -1)
+ die(seek failed);
}
gettimeofday(elapse_t, NULL);
close(tmpfile);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Adding \ev view editor?
Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: That's not what I had in mind by decoupling the option's effects. Well, regardless of that it does what I want, and with a fairly small amount of code. Well, yeah, because you are paying no mind to what anyone else might want. I can make it work only in the pretty print case, if that's your objection. I think that's back where we started in this thread. I can live with it, but I thought some other people were unhappy with the idea of many lines of targetlist... 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] [rfc] unicode escapes for extended strings
On Wed, 2009-09-09 at 18:26 +0300, Marko Kreen wrote: Unicode escapes for extended strings. On 4/16/09, Marko Kreen mark...@gmail.com wrote: Reasons: - More people are familiar with \u escaping, as it's standard in Java/C#/Python, probably more.. - U strings will not work when stdstr=off. Syntax: \u - 16-bit value \U - 32-bit value Additionally, both \u and \U can be used to specify UTF-16 surrogate pairs to encode characters with value 0x. This is exact behaviour used by Java/C#/Python. (except that Java does not have \U) v3 of the patch: - convert to new reentrant lexer API - add lexer targets to avoid fallback to default - completely disallow \U\u without proper number of hex values - fix logic bug in surrogate pair handling This looks good to me. I'm implementing the surrogate pair handling for the U syntax for consistency. Then I'll apply this. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic copy options
Tom Lane t...@sss.pgh.pa.us writes: Applied with revisions as discussed. Excellent ;) Now if you wanted a small option to play with to test the extensibility of the new system, should I propose DEFAULT '\D' (e.g.)? Regards, -- dim -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Adding \ev view editor?
Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: That's not what I had in mind by decoupling the option's effects. Well, regardless of that it does what I want, and with a fairly small amount of code. Well, yeah, because you are paying no mind to what anyone else might want. Umm, no. I have listened but it's not clear what people want. I don't hear too many people actually defending the status quo. Any viewdef with any substantial number of columns is just about unreadable. I can make it work only in the pretty print case, if that's your objection. I think that's back where we started in this thread. I can live with it, but I thought some other people were unhappy with the idea of many lines of targetlist... Well, some were yes, but nobody actually came up with anything better. IIRC the code I tried (in response to what people said) that limited newlines by only wrapping lines at some limit got more objections than my original proposal. At one stage you said: Why don't you have the flag just driving your original two-line addition? The patch I sent today is more or less the equivalent of that two-line addition, but removes some ugliness. It's a few more lines, but not many. So that suggestion of mine above to make it work just in the pretty print case was actually intended to be responsive to your previous suggestion. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
--On 21. September 2009 13:42:21 +0200 Bernd Helmle maili...@oopsware.de wrote: --On 20. September 2009 22:56:53 -0400 Robert Haas robertmh...@gmail.com wrote: So is this ready to commit, or what? Not yet, see the comments Alvaro did upthread. Please note that i'm still reviewing this one and i hope to post results tomorrow (there wasn't plenty of free time over the weekend, i'm sorry). Here some further comments on the current patch: - I'm not sure i like the name of the new system catalog pg_setting. Wie already have pg_settings, i think this can be confusing. Maybe we need a different name, e.g. pg_user_setting? This seems along the line with the other *user* system objects (e.g. pg_stat_user_tables), where only user specific objects are displayed. - I have thought a little bit about the changes in the system views. pg_roles and pg_shadow (as Alvaro already mentioned), need to be adjusted (joined to the new catalog), to display rolconfig/useconfig. However, it's unclear *how* to expose those information, for example, do we want to expose roleconfig specific for the current database or for all databases the role has a specific config for ? - The code mentions the lack of lock synchronization. Maybe i'm missing something obvious (its late here), but is there a reason this can't be done by obtaining a lock on pg_authid (not sure about the backend user initialization phase though) ? - Regarding the missing UI: i go with Alvaro's proposal: ALTER ROLE rolename [ALTER] DATABASE dbname SET config TO value; Maybe we can make the 2nd ALTER optional. Thoughts? -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
On Mon, Sep 21, 2009 at 12:23 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Robert Haas escribió: So here's the followup question - do you intend to do one of those things for this CommitFest, or should we mark this as Returned with Feedback once Bernd posts the rest of his review? What feedback is it supposed to be returned with? Precisely what I wanted is some feedback on the general idea. Brendan's I like the approach is good, but is it enough to deter a later veto from someone else? Well, you've hit there on one of the things that we don't always do well. Many a patch author has posted an idea, received no feedback, proceeded to implementation, and then the knives come out. On a good day, the CommitFest process ensures that every patch gets a second opinion, but it doesn't guarantee that a third opinion won't come crawling out of the woodwork at a later date. In this respect, you're actually operating at a slight advantage relative to most of us, because you can post your revised patch and commit it if no one objects too strongly, whereas I (for example) have to convince one of about two people - Tom or Peter, for nearly anything I'm likely to develop - to take an affirmative action on my behalf. This whole phenomenon of proposals to which no objection was made at the outset later getting flak for one reason or another is, I think, a source of much frustration and discourages people from putting effort into projects they might otherwise be willing to undertake. But I haven't the least idea how to fix it, and I can't offer you any guarantees with respect to the present situation either. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] updated hstore patch
David == David E Wheeler da...@kineticode.com writes: But I checked, and delete(hstore,$1) still resolves to delete(hstore,text) when the type of $1 is not specified, so there's no compatibility issue there that I can see. (I'm not sure I understand _why_ it resolves to that rather than being ambiguous...) David Right, but it does seem like it might be the best choice for David now. I'd add a regression test to make sure it stays that way. I don't think there's any way to do that from the regression tests. -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] updated hstore patch
On Sep 21, 2009, at 4:57 PM, Andrew Gierth wrote:
I don't think there's any way to do that from the regression tests.
The output that you demonstrated a few messages back should do nicely
for delete(), at least:
contrib_regression=# explain verbose select delete(('a' =
now()::text),'a');
QUERY PLAN
---
Result (cost=0.00..0.02 rows=1 width=0)
Output: delete(('a'::text = (now())::text), 'a'::text)
(2 rows)
contrib_regression=# explain verbose select delete(('a' =
now()::text),'a=1'::hstore);
QUERY PLAN
Result (cost=0.00..0.02 rows=1 width=0)
Output: delete(('a'::text = (now())::text), 'a=1'::hstore)
(2 rows)
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
Re: [HACKERS] updated hstore patch
David E. Wheeler da...@kineticode.com writes: On Sep 21, 2009, at 4:57 PM, Andrew Gierth wrote: I don't think there's any way to do that from the regression tests. The output that you demonstrated a few messages back should do nicely for delete(), at least: Anything involving 'explain verbose' output is not getting into the regression tests. It's not stable enough. In practice, I don't see why simply testing the result of the function isn't enough for this... 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] updated hstore patch
Tom == Tom Lane t...@sss.pgh.pa.us writes:
On Sep 21, 2009, at 4:57 PM, Andrew Gierth wrote:
I don't think there's any way to do that from the regression tests.
The output that you demonstrated a few messages back should do nicely
for delete(), at least:
Tom Anything involving 'explain verbose' output is not getting into the
Tom regression tests. It's not stable enough.
Tom In practice, I don't see why simply testing the result of the
Tom function isn't enough for this...
Is delete('...'::hstore,'foo') guaranteed to resolve to the same
function as delete('...'::hstore,$1) where $1 has no type specified?
If so, then the regression tests already cover this.
--
Andrew (irc:RhodiumToad)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] updated hstore patch
Andrew Gierth and...@tao11.riddles.org.uk writes:
Is delete('...'::hstore,'foo') guaranteed to resolve to the same
function as delete('...'::hstore,$1) where $1 has no type specified?
Yup. They're both UNKNOWN.
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] [PATCH] pgbench: new feature allowing to launch shell commands
Hi, Sorry for my late reply again :o) You will find my answers on-the-line. You really should be returning a value at the point since the function signature defines a return type. If not the function should be void, which it cannot be in this context since it is used for boolean tests elsewhere. The returns in question are all part of error blocks and should return false. The function doCustom is defined with a void. I agree that it is far cleaner to return a value but by returning in my own code part a boolean or an integer, it will have a large impact on other parts of the code that are dealing with the original command types of pgbench. By the way, the errors created by doCustom are managed through CState for each customer, so letting it like it is is far sufficient and has no impact on other parts of the code. So I get this output with and with out the shell command in there, against the unpatched and patched version on pgbench. The commands you sent will not work with this script since it is using prepared statements. I am using this command to run the script. I made on my side a couple of tests to see what is happening in the code. The problem you saw is not linked to the patch I wrote. In fact when you are using the prepared statement mode, PQprepare seems not to be able to manage with the parameter I put in my transaction script at prepare transaction and commit prepared stages. This parameter is the random ID used for prepared transaction. If you try an end script such as: PREPARE TRANSACTION 'T'; \shell ls -ll ~/pgsql/data COMMIT PREPARED 'T'; It will work without any problem in both prepared and single query mode. For 1 customer, there is no issue. However, by increasing the number of customers, it will increase the risk for a customer to prepare a transaction who is using a same 2pc ID, resulting in a couple of output errors. Using the original script I wrote is OK in single query mode, as 2pc ID is not managed by PQprepare but at pgbench level. This way pgbench will not create errors, as it builds an individual query with a random ID one by one. The prepared query mode prepares the same transaction for all the customers of pgbench, and I think that PQprepare cannot manage 2pc IDs while preparing a transaction. Parameters in SQL queries is OK, but not at prepare states. That would explain why in this case, the system was looking for a parameter that is not delivered initially. Besides, you can also make tests without 2pc transactions, such as: \shell ls -ll /home/ioltas/usr/pgsql/data END; In this case also it works finely in both prepared and single query mode (at least on my side :)). By looking at the documentation of pgbench, prepared query mode is used for performance purposes only. The pgbench shell feature tends to slow down all the process as it has been created for analysis purposes only, so the single query mode is enough to my mind if you want to study the interactions of your system and postgres. Regards, Michael
Re: [HACKERS] Hot Standby 0.2.1
Simon Riggs wrote: OK, here is the latest version of the Hot Standby patchset. This is about version 30+ by now, but we should regard this as 0.2.1 Patch against CVS HEAD (now): clean apply, compile, no known bugs. Wow, great! Simon has allowed us to pass a great milestone in Postgres development. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Standalone backends run StartupXLOG in an incorrect environment
On Monday 21 September 2009 14:24:07 Tom Lane wrote: Alvaro Herrera alvhe...@commandprompt.com writes: So if you need to enter standalone mode, you'll have to start postmaster, wait for replay to finish, stop it and restart standalone. Yeah, that's the case at the moment. Would this be a problem when you need standalone mode in an emergency, for example when the database won't start due to Xid wraparound? If it won't run through StartupXLOG under the postmaster, it's not going to do so standalone either. Hmm... istr cases where I couldn't startup regular postgres but could in stand-alone mode that had system indexes disabled...I could be misremembering that so that the postmaster would start, I just couldn't connect unless in stand-alone. In any case this does seem less than ideal, but if there aren't any better options... -- Robert Treat Conjecture: http://www.xzilla.net Consulting: http://www.omniti.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] Using results from INSERT ... RETURNING
On Sun, Sep 6, 2009 at 6:10 AM, Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: Fixed a couple of bugs and renovated ExecInitDml() a bit. Patch attached. Hi, I'm reviewing this patch for this CommitFest. With regard to the changes in explain.c, I think that the way you've capitalized INSERT, UPDATE, and DELETE is not consistent with our usual style for labelling nodes. Also, you've failed to set sname, so this reads from uninitialized memory when using JSON or XML format. I think that you should handle XML/JSON format by setting sname to Dml and then emit an operation field down around where we do if (strategy) ExplainPropertyText(Strategy, ...). I am not sure that I like the name Dml for the node type. Most of our node types are descriptions of the action that will be performed, like Sort or HashJoin; Dml is the name of the feature we're trying to implement, but it's not the name of the action we're performing. Not sure what would be better, though. Write? Modify? Can you explain the motivation for changing the Append stuff as part of this patch? It's not immediately clear to me why that needs to be done as part of this patch or what we get out of it. What is your general impression about the level of maturity of this code? Are you submitting this as complete and ready for commit, or is it a WIP? If the latter, what are the known issues? I'll try to provide some more feedback on this after I look it over some more. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby 0.2.1
On Tue, Sep 15, 2009 at 2:41 PM, Simon Riggs si...@2ndquadrant.com wrote: OK, here is the latest version of the Hot Standby patchset. This is about version 30+ by now, but we should regard this as 0.2.1 Patch against CVS HEAD (now): clean apply, compile, no known bugs. OVERVIEW You can download PDF versions of the fine manual is here http://wiki.postgresql.org/images/0/01/Hot_Standby_main.pdf From this doc: In recovery, transactions will not be permitted to take any lock higher other than AccessShareLock or AccessExclusiveLock. In addition, transactions may never assign a TransactionId and may never write WAL. The LOCK TABLE command by default applies an AccessExclusiveLock. Any LOCK TABLE command that runs on the standby and requests a specific lock type other than AccessShareLock will be rejected. The first sentence seems to say that clients on the stand-by can take ACCESS EXCLUSIVE, while the last sentence seems to say that they cannot do so. I did a little experiment on a hot standby instance. I expected that either I would be denied the lock altogether, or the lock would cause WAL replay to be paused until either I committed or was forcibly canceled. But neither happened, I was granted the lock but WAL replay continued anyway. jjanes=# begin; BEGIN jjanes=# lock table pgbench_history in access exclusive mode; LOCK TABLE jjanes=# select count(*) from pgbench_history; count 519104 (1 row) jjanes=# select count(*) from pgbench_history; count 527814 (1 row) Is this the expected behavior? Thanks, Jeff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
On Tue, Sep 22, 2009 at 4:16 AM, Bernd Helmle maili...@oopsware.de wrote:
--On 21. September 2009 13:42:21 +0200 Bernd Helmle maili...@oopsware.de
wrote:
--On 20. September 2009 22:56:53 -0400 Robert Haas
robertmh...@gmail.com wrote:
So is this ready to commit, or what?
Not yet, see the comments Alvaro did upthread. Please note that i'm still
reviewing this one and i hope to post results tomorrow (there wasn't
plenty of free time over the weekend, i'm sorry).
Here some further comments on the current patch:
- I'm not sure i like the name of the new system catalog pg_setting. Wie
already have pg_settings, i think this can be confusing. Maybe we need a
different name, e.g. pg_user_setting? This seems along the line with the
other *user* system objects (e.g. pg_stat_user_tables), where only user
specific objects are displayed.
- I have thought a little bit about the changes in the system views.
pg_roles and pg_shadow (as Alvaro already mentioned), need to be adjusted
(joined to the new catalog), to display rolconfig/useconfig. However, it's
unclear *how* to expose those information, for example, do we want to expose
roleconfig specific for the current database or for all databases the role
has a specific config for ?
- The code mentions the lack of lock synchronization. Maybe i'm missing
something obvious (its late here), but is there a reason this can't be done
by obtaining a lock on pg_authid (not sure about the backend user
initialization phase though) ?
- Regarding the missing UI: i go with Alvaro's proposal:
ALTER ROLE rolename [ALTER] DATABASE dbname SET config TO value;
Maybe we can make the 2nd ALTER optional.
Thoughts?
ON instead of second ALTER looks better, and IMHO DATABASE dbname should
be optional too:
ALTER ROLE rolename [ON DATABASE dbname] SET config TO value;
Best regards,
--
Lets call it Postgres
EnterpriseDB http://www.enterprisedb.com
gurjeet[.sin...@enterprisedb.com
singh.gurj...@{ gmail | hotmail | indiatimes | yahoo }.com
Twitter: singh_gurjeet
Skype: singh_gurjeet
Mail sent from my BlackLaptop device
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
Gurjeet Singh singh.gurj...@gmail.com writes: ON instead of second ALTER looks better, and IMHO DATABASE dbname should be optional too: ALTER ROLE rolename [ON DATABASE dbname] SET config TO value; IN, not ON. 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] numeric_to_number() function skipping some digits
Hi,
On Mon, Sep 21, 2009 at 12:36 PM, Brendan Jurd dire...@gmail.com wrote:
2009/9/21 Jeevan Chalke jeevan.cha...@enterprisedb.com:
Oracle returns 19-SEP-09 irrespective of the format.
Here in PG, we have getting the proper date irrespective of the format as
Oracle. But in the case to to_number the returned value is wrong. For
example following query returns '340' on PG where as it returns '3450' on
Oracle.
select to_number('34,50','999,99') from dual;
Hi Jeevan,
Thanks for checking up on the Oracle behaviour. It appears to
silently disregard grouping characters in the format pattern, and also
disregard them wherever they appear in the input string (or else it
reads the string from right-to-left?).
It seems that Oracle reads formatting string from right-to-left. Here are
few results:
('number','format') == Oracle PG
('34,50','999,99') == 3450340
('34,50','99,99') == 34503450
('34,50','99,999') == Invalid Number 3450
('34,50','999,999') == Invalid Number 340
It seems that, to match Oracle, we'd need to teach the code that 'G'
and ',' are no-ops for to_number(), and also that such characters
should be ignored in the input.
That means we cannot simply ignore such characters from the input. Rather we
can process the string R-L. But yes this will definitely going to break the
current applications running today.
To be honest, though, I'm not sure it's worth pursuing. If you want
to feed in numbers that have decorative characters all through them,
it's far more predictable to just regex out the cruft and use ordinary
numeric parsing than to use to_number(), which is infamous for its
idiosyncrasies:
# SELECT regexp_replace('34,50', E'[\\d.]', '', 'g')::numeric;
3450
This (with E'[^\\d.]') ignores/replaces all the characters except digits
from the input which we certainly not wishing to do. Instead we can continue
with the current implementation. But IMHO, somewhere in the time-line we
need to fix this.
Cheers,
BJ
Thanks
--
Jeevan B Chalke
EnterpriseDB Software India Private Limited, Pune
Visit us at: www.enterprisedb.com
---
If better is possible, then good is not enough
