Re: [HACKERS] INSERT ... ON CONFLICT () SELECT
On Sun, Jun 18, 2017 at 9:21 PM, Peter Geoghegan wrote: > Returning rows with duplicate values seems rather unorthodox. > Ok, then option 2 it is. In summary, this is what I am going to (attempt to) implement for the new syntax: INSERT ... ON CONFLICT (...) DO SELECT RETURNING ... 1. Rows that are in conflict are made available to the RETURNING clause. In other words, it is like an idempotent "ON CONFLICT DO UPDATE". 2. Similarly, insertion sets that would cause the error "ON CONFLICT DO UPDATE command cannot affect row a second time" if it were an "ON CONFLICT DO UPDATE" statement will also cause a similar error for "ON CONFLICT DO SELECT". This will prevent duplicate rows from being returned. 3. Like an "ON CONFLICT DO UPDATE", the returned rows cannot be changed by another part of the wCTE, even if no actual insertions occurred. Unless I have missed anything, I think all other issues have been adequately addressed. Since there are no red lights, I shall proceed. :) Best regards, Matt
Re: [HACKERS] INSERT ... ON CONFLICT () SELECT
On Sat, Jun 17, 2017 at 9:55 PM, Peter Geoghegan wrote:
> On Sat, Jun 17, 2017 at 7:49 AM, Matt Pulver
> wrote:
> > With the proposed "INSERT ... ON CONFLICT () SELECT" feature, the
> > get_or_create_id() function is simplified to:
>
> Are you locking the existing rows? Because otherwise, the
> determination that they're conflicting can become obsolete immediately
> afterwards. (I guess you would be.)
>
If that is required in order to return the rows in their conflicted state,
then yes.
> The problem with this design and similar designs is that presumably
> the user is sometimes projecting the conflicting rows with the
> intention of separately updating them in a wCTE. That might not work,
> because only ON CONFLICT doesn't use the MVCC snapshot, in order to
> ensure that an UPDATE is guaranteed when an INSERT cannot go ahead.
> That isn't what you're doing in the example you gave, but surely some
> users would try to do things like that, and get very confused.
>
Ultimately the proposed "INSERT ... ON CONFLICT () DO SELECT" syntax is
still an INSERT statement, not a SELECT, so a user should not expect rows
returned from it to be available for UPDATE/DELETE in another part of a
wCTE. Anyone who understands this behavior for an INSERT statement, let
alone the current "INSERT ... ON CONFLICT DO UPDATE" should not be too
surprised if the same thing applies to the new "INSERT ... ON CONFLICT DO
SELECT".
> I think that what you propose to do here would likely create a lot of
> confusion by mixing MVCC semantics with special UPSERT visibility
> semantics ("show me the latest row version visible to any possible
> snapshot for the special update") even without a separate UPDATE, in
> fact. Would you be okay if "id" appeared duplicated in the rows you
> project in your new syntax, even when there is a separate unique
> constraint on that column? I suppose that there is some risk of things
> like that today, but this would make the "sleight of hand" used by ON
> CONFLICT DO UPDATE more likely to cause problems.
>
Good point. Here is an example using the example table from my previous
email:
INSERT INTO example (name) VALUES ('foo'), ('foo')
ON CONFLICT (name) DO SELECT
RETURNING *
Here are a couple options of how to handle this:
1) Return two identical rows (with the same id).
2) Produce an error, with error message:
"ERROR: ON CONFLICT DO SELECT command cannot reference row a second time
HINT: Ensure that no rows proposed for insertion within the same command
have duplicate constrained values."
This would be nearly identical to the existing error message that is
produced when running:
INSERT INTO example (name) VALUES ('foo'), ('foo')
ON CONFLICT (name) DO UPDATE SET value=1
RETURNING *
which gives the error message:
"ERROR: ON CONFLICT DO UPDATE command cannot affect row a second time
HINT: Ensure that no rows proposed for insertion within the same command
have duplicate constrained values."
Technically, an error doesn't *need* to be produced for the above "ON
CONFLICT DO SELECT" statement - I think it is still perfectly well-defined
to return duplicate rows as in option 1. Option 2 is more for the benefit
of the user who is probably doing something wrong by attempting to INSERT a
set of rows that violate a constraint. What do you think would be best?
Thank you for the discussion.
Best regards,
Matt
[HACKERS] INSERT ... ON CONFLICT () SELECT
Hello,
I am looking to add a new language feature that returns the rows that
conflict on an INSERT, and would appreciate feedback and guidance on this.
Here is an example.
To implement a get_or_create_id() function, this is how it must currently
be done:
CREATE TABLE example (
id SERIAL PRIMARY KEY,
name TEXT NOT NULL,
value FLOAT);
CREATE UNIQUE INDEX ON example (name);
CREATE FUNCTION get_or_create_id(_name TEXT) RETURNS INT AS
$$
WITH get AS (
SELECT id FROM example WHERE name=_name
), new AS (
INSERT INTO example (name) VALUES (_name)
ON CONFLICT (name) DO NOTHING
RETURNING id
)
SELECT id FROM get
UNION ALL
SELECT id FROM new
$$
LANGUAGE sql;
SELECT get_or_create_id('foo'); -- 1
SELECT get_or_create_id('bar'); -- 2
SELECT get_or_create_id('foo'); -- 1
With the proposed "INSERT ... ON CONFLICT () SELECT" feature, the
get_or_create_id() function is simplified to:
CREATE FUNCTION get_or_create_id(_name TEXT) RETURNS INT AS
$$
INSERT INTO example (name) VALUES (_name)
ON CONFLICT (name) DO SELECT
RETURNING id
$$
LANGUAGE sql;
In the case of a CONFLICT, the selected rows are exactly those same rows
that would be operated on by an ON CONFLICT () DO UPDATE clause. These rows
are then made available to the RETURNING clause in the same manner. Just
like "DO NOTHING", the "DO SELECT" clause takes no arguments. It only makes
the conflicting rows available to the RETURNING clause.
Tom Lane has previously responded
<https://www.postgresql.org/message-id/19806.1468429...@sss.pgh.pa.us> to a
similar request which was ill-defined, especially in the context of
exclusion constraints. I believe that by SELECTing exactly those same rows
that an UPDATE clause would on a CONFLICT, this becomes well-defined, even
with exclusion constraints.
Feedback/guidance is most welcome.
Best regards,
Matt
Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON
Its worth noting that the JDBC's behavior when you switch back to autocommit is to immediately commit the open transaction. Personally, I think committing immediately or erroring are unsurprising behaviors. The current behavior is surprising and obviously wrong. Rolling back without an error would be very surprising (no other database API I know of does that) and would take forever to debug issues around the behavior. And committing after the next statement is definitely the most surprising behavior suggested. IMHO, I think committing immediately and erroring are both valid. I think I'd prefer the error in principle, and per the law of bad code I'm sure, although no one has ever intended to use this behavior, there is probably some code out there that is relying on this behavior for "correctness". I think a hard failure and making the dev add an explicit commit is least likely to cause people serious issues. As for the other options, consider me opposed. - Matt K.
Re: [HACKERS] Cluster on NAS and data center.
As someone who has bitten by index corruption due to collation changes between glibc versions that shipped CentOS 6 and CentOS 7, don't even try to do this with anything other than C collation. The default collation _will_ deterministically leave you with a silently corrupt database if you store anything other than ASCII text. Windows and Linux are going to implement en_US.utf-8 slightly differently and Postgres is currently relying on the OS to provide collation implementations. Go search for my mailing list post about the dangers of running across versions of glibc for more info. I'm going to echo everyone else's sentiment though, and assert that what you are trying to do is really an insane idea. You might be able to make it appear like its working but as a DBA, I would have absolutely no confidence in using that server for disaster recovery. If your company is saving money by not getting Windows licenses for your DR environment, you are far better off just saving one more license and making both your production and DR server be Linux builds. - Matt K.
Re: [HACKERS] btree_gin and btree_gist for enums
Hi Andrew, all, First message here! I didn't get around to sending an intro/"thank you all" e-mail yet, and a small performance (?) patch+idea(s)... (CPU stuff, since I don't otherwise know much about PG internals.) Anyway... - Original Message - From: "Andrew Dunstan" Sent: Thursday, March 17, 2016 Here is a patch to add enum support to btree_gin and btree_gist. I didn't include distance operations, as I didn't think it terribly important, and there isn't a simple way to compute it sanely and efficiently, so no KNN support. Major thanks for coming up with this a week after your "enums and indexing" message. My love of all things Postgres has a lot to do with being able to do nearly anything one could want (coming from MySQL :-/)! So I was like, "Wait, what?" when you brought up the subject, since this was something I hadn't actually tried, but was planning to use btree_gin/gist with a good mix of stuff, including enums (array and scalar). At first I thought it was just arrays of enums, until this patch for the contrib extensions (for the btree parts with GIN/GiST; plus GiST exclusion?), and your blog posts... [1][2] I wasn't certain from the second post whether your array solution can be used today without this patch (yes, just tried 9.5). So, two separate issues, and the patch addresses scalar stuff, IIUC. It would be *really* nice to have this in 9.6. It seems it's simply filling out functionality that should already be there, right? An oversight/bug fix so it works "as advertised?" :-) Is any other btree type-compatibility missing from these modules? (I notice the btree_gin docs don't mention "numeric," but it works.) I just looked over the patch, and the actual code addition/changes seem pretty small and straightforward (or am I wrong?). And it's not changing anything in the core, so... Well, just wanted to argue my case! :^) [1] http://adpgtech.blogspot.com/2016/03/gist-and-gin-support-for-enums.html [2] http://adpgtech.blogspot.com/2016/03/gin-indexing-array-of-enums.html cheers andrew Thanks, Matt -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] amcheck (B-Tree integrity checking tool)
> > On Fri, 2016-03-11 at 17:24 +0100, Michael Paquier wrote: > > On Fri, Mar 11, 2016 at 5:19 PM, Anastasia Lubennikova wrote: > > > > > > BTW, if you know a good way to corrupt index (and do it > > > reproducible) I'd be > > > very glad to see it. > > You can use for example dd in non-truncate mode to corrupt on-disk > > page data, say that for example: > > dd if=/dev/random bs=8192 count=1 \ > > seek=$BLOCK_ID of=base/$DBOID/$RELFILENODE \ > > conv=notrunc > I guess /dev/random is not very compatible with the "reproducible" > requirement. I mean, it will reproducibly break the page, but pretty > much completely, which is mostly what checksums are for. You can actually pretty easily produce a test case by setting up streaming replication between servers running two different version of glibc. I actually wrote a tool that spins up a pair of VMs using vagrant and then sets them up as streaming replica's using ansible. It provides a nice one liner to get a streaming replica test environment going and it will easily provide the cross glibc test case. Technically, though it belongs to Trip because I wrote it on company time. Let me see if I can open source a version of it later this week that way you can use it for testing. - Matt K.
Re: [HACKERS] LISTEN denial of service with aborted transaction
>
> > After further thought, I wonder whether instead of having an incoming
> > listener initialize its "pos" to QUEUE_TAIL, we couldn't have it
> > initialize to the maximum "pos" among existing listeners (with a floor of
> > QUEUE_TAIL of course). If any other listener has advanced over a given
> > message X, then X was committed (or aborted) earlier and the new listener
> > is not required to return it; so it should be all right to take that
> > listener's "pos" as a starting point.
>
> That's a great idea and I will try it. It does need to be the maximum
> position of existing listeners *with matching db oid" since
> asyncQueueReadAllNotifications doesn't check transaction status if the db
> oid doesn't match it's own. Another advantage of this approach is that a
> queued notify from an open transaction in one database won't incur
> additional cost on listens coming from other databases, whereas with my
> patch such a situation will prevent firstUncommitted from advancing.
>
Patch attached works great. I added the dboid to the QueueBackendStatus
struct but that might not be needed if there is an easy and fast way to get a
db oid from a backend pid.
I also skip the max pos calculation loop if QUEUE_HEAD is on the same page as
QUEUE_TAIL, with the thought that it's not desirable to increase the time that
AsyncQueueLock is held if the queue is small and
asyncQueueReadAllNotifications will execute quickly.
I then noticed that we are taking the same lock twice in a row to read the
same values, first in Exec_ListenPreCommit then in
asyncQueueReadAllNotifications, and much of the time the latter will simply
return because pos will already be at QUEUE_HEAD. I prepared a second patch
that splits asyncQueueReadAllNotifications. Exec_ListPreCommit then only calls
the worker version only when needed. This avoids the duplicate lock.
Thanks,
Matt Newelldiff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 91baede..58682dc 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -202,6 +202,12 @@ typedef struct QueuePosition
(x).page != (y).page ? (y) : \
(x).offset < (y).offset ? (x) : (y))
+/* choose logically larger QueuePosition */
+#define QUEUE_POS_MAX(x,y) \
+ (asyncQueuePagePrecedes((x).page, (y).page) ? (y) : \
+ (x).page != (y).page ? (x) : \
+ (x).offset < (y).offset ? (y) : (x))
+
/*
* Struct describing a listening backend's status
*/
@@ -209,6 +215,7 @@ typedef struct QueueBackendStatus
{
int32 pid; /* either a PID or InvalidPid */
QueuePosition pos; /* backend has read queue up to here */
+ Oid dboid; /* backend's database OID */
} QueueBackendStatus;
/*
@@ -248,6 +255,7 @@ static AsyncQueueControl *asyncQueueControl;
#define QUEUE_TAIL (asyncQueueControl->tail)
#define QUEUE_BACKEND_PID(i) (asyncQueueControl->backend[i].pid)
#define QUEUE_BACKEND_POS(i) (asyncQueueControl->backend[i].pos)
+#define QUEUE_BACKEND_DBOID(i) (asyncQueueControl->backend[i].dboid)
/*
* The SLRU buffer area through which we access the notification queue
@@ -461,6 +469,7 @@ AsyncShmemInit(void)
for (i = 0; i <= MaxBackends; i++)
{
QUEUE_BACKEND_PID(i) = InvalidPid;
+ QUEUE_BACKEND_DBOID(i) = InvalidOid;
SET_QUEUE_POS(QUEUE_BACKEND_POS(i), 0, 0);
}
}
@@ -907,6 +916,9 @@ AtCommit_Notify(void)
static void
Exec_ListenPreCommit(void)
{
+ QueuePosition max;
+ int i;
+
/*
* Nothing to do if we are already listening to something, nor if we
* already ran this routine in this transaction.
@@ -936,7 +948,25 @@ Exec_ListenPreCommit(void)
* doesn't hurt.
*/
LWLockAcquire(AsyncQueueLock, LW_SHARED);
- QUEUE_BACKEND_POS(MyBackendId) = QUEUE_TAIL;
+ max = QUEUE_TAIL;
+ /*
+ * If there is over a page of notifications queued, then we should find
+ * the maximum position of all backends connected to our database, to
+ * avoid having to process all of the notifications that belong to already
+ * committed transactions that we will disregard anyway. This solves
+ * a significant performance problem with listen when there is a long
+ * running transaction
+ */
+ if( QUEUE_POS_PAGE(max) < QUEUE_POS_PAGE(QUEUE_HEAD) )
+ {
+ for (i = 1; i <= MaxBackends; i++)
+ {
+ if (QUEUE_BACKEND_PID(i) != InvalidPid && QUEUE_BACKEND_DBOID(i) == MyDatabaseId)
+max = QUEUE_POS_MAX(max, QUEUE_BACKEND_POS(i));
+ }
+ }
+ QUEUE_BACKEND_POS(MyBackendId) = max;
+ QUEUE_BACKEND_DBOID(MyBackendId) = MyDatabaseId;
QUEUE_BACKEND_PID(MyBackendId) = MyProcPid;
LWLockRelease(AsyncQueueLock);
@@ -1156,6 +1186,7 @@ asyncQueueUnregister(void)
QUEUE_POS_EQUAL(QUEUE_BACKEND_POS(MyBackendId), QUEUE_TAIL);
/* ... then mark it invalid */
QUEUE_BACKEND_PID(MyBackendId) = InvalidPid;
+ QUEUE_BACKEND_DBOID(
Re: [HACKERS] LISTEN denial of service with aborted transaction
On Tuesday, September 29, 2015 09:58:35 PM Tom Lane wrote: > Matt Newell writes: > > On Monday, September 28, 2015 07:22:26 PM Tom Lane wrote: > >> Possibly. sinval catchup notification works like that, so you could > >> maybe > >> invent a similar mechanism for notify. Beware that we've had to fix > >> performance issues with sinval catchup; you don't want to release a whole > >> bunch of processes to do work at the same time. > > > > I'll have to think about this more but i don't envision it causing such as > > scenario. > > The extra processing that will happen at signaling time would have been > > done anyway when the aborted transaction is finally rolled back, the only > > extra work is copying the relevant notifications to local memory. > > Hm. An idle-in-transaction listening backend will eventually cause a more > serious form of denial of service: once the pg_notify SLRU area fills up > to its maximum of 8GB, no more new messages can be inserted, and every > transaction that tries to do a NOTIFY will abort. However, that's a > documented hazard and we've not really had complaints about it. In any > case, trying to fix that by having such a backend absorb messages into > local memory doesn't seem like a great idea: if it sits for long enough, > its local memory usage will bloat. Eventually you'd have a failure > anyway. > > > Regardless it might not be worthwhile since my patch for #1 seems to > > address the symptom that bit me. > > I looked at this patch for a bit and found it kind of ugly, particularly > the business about "we allow one process at a time to advance > firstUncommitted by using advancingFirstUncommitted as a mutex". > That doesn't sound terribly safe or performant. > It can be implemented without that but then you'll have multiple backends trying to do the same work. This might not be an issue at all but I couldn't tell at first glance the potential cost of extra calls to TransactionIdIsInProgress. Since there are no extra locks taken I figured ensuring the work is only done once is good for performance. If the cluster only has one database generating notifies then there is practically no extra work with the patch. As far as safety is concerned I think the worst case is that behavior returns to what it is now, where firstUncommitted ends up tracking QUEUE_TAIL. I originally used a boolean then realized I could get some extra safety by using the backendId so that the mutex would be released automatically if the backend dies. > After further thought, I wonder whether instead of having an incoming > listener initialize its "pos" to QUEUE_TAIL, we couldn't have it > initialize to the maximum "pos" among existing listeners (with a floor of > QUEUE_TAIL of course). If any other listener has advanced over a given > message X, then X was committed (or aborted) earlier and the new listener > is not required to return it; so it should be all right to take that > listener's "pos" as a starting point. That's a great idea and I will try it. It does need to be the maximum position of existing listeners *with matching db oid" since asyncQueueReadAllNotifications doesn't check transaction status if the db oid doesn't match it's own. Another advantage of this approach is that a queued notify from an open transaction in one database won't incur additional cost on listens coming from other databases, whereas with my patch such a situation will prevent firstUncommitted from advancing. > > The minimum-code-change way to do that would be to compute the max pos > the hard way while Exec_ListenPreCommit holds the AsyncQueueLock, which > it would now have to do in exclusive mode. That's a little bit annoying > but it's surely not much worse than what SignalBackends has to do after > every notification. Yeah I think it's going to be a win regardless. Could also skip the whole thing if QUEUE_HEAD - QUEUE_TAIL is under a fixed amount, which would probably cover a lot of use cases. > > Alternatively, we could try to maintain a shared pointer that is always > equal to the frontmost backend's "pos". But I think that would require > more locking during queue reading than exists now; so it's far from clear > it would be a win, especially in systems where listening sessions last for > a reasonable amount of time (so that Exec_ListenPreCommit is infrequent). And that would be even more complicated since it has to be per db oid. Matt Newell -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LISTEN denial of service with aborted transaction
On Monday, September 28, 2015 07:22:26 PM Tom Lane wrote: > Matt Newell writes: > > 1. When a connection issues it's first LISTEN command, in > > Exec_ListenPreCommit QUEUE_BACKEND_POS(MyBackendId) = QUEUE_TAIL; > > this causes the connection to iterate through every notify queued in the > > slru, even though at that point I believe the connection can safely > > ignore any notifications from transactions that are already committed, > > and if I understand correctly notifications aren't put into the slru > > until precommit, so wouldn't it be safe to do: > > QUEUE_BACKEND_POS(MyBackendId) = QUEUE_HEAD; > > inside Async_Listen? > > Per the comment in Exec_ListenPreCommit, the goal here is to see entries > that have been made and not yet committed. If you don't do it like this, > then a new listener has the possibility of receiving notifications from > one transaction, while not seeing notifications from another one that > committed later, even though longer-established listeners saw outputs from > both transactions. We felt that that sort of application-visible > inconsistency would be a bad thing. > Right, QUEUE_HEAD can't be used, however... > > If that's not safe, then could a new member be added to > > AsyncQueueControl that points to the first uncommitted QueuePosition > > (wouldn't have to be kept completely up to date). > > Err ... isn't that QUEUE_TAIL already? I've not studied this code in > detail recently, though. > QUEUE_TAIL is the oldest notification. My idea is to keep a somewhat up-to- date pointer to the oldest uncommitted notification, which can be used as a starting point when a connection issues it's first listen. Just as the current code will advance a backend's pointer past any committed notifications when it calls asyncQueueReadAllNotifications in Exec_ListenPreCommit with no registered listeners. I came up with a proof of concept and it appears to work as expected. With 500,000 notifications queued a listen is consistently under .5ms, while my 9.4.4 install is taking 70ms, and the speedup should be much greater on a busy server where there is more lock contention. Attached is the patch and the ugly test script. > > 2. Would it be possible when a backend is signaled to check if it is idle > > inside an aborted transaction, and if so process the notifications and put > > any that match what the backend is listening on into a local list. > > Possibly. sinval catchup notification works like that, so you could maybe > invent a similar mechanism for notify. Beware that we've had to fix > performance issues with sinval catchup; you don't want to release a whole > bunch of processes to do work at the same time. I'll have to think about this more but i don't envision it causing such as scenario. The extra processing that will happen at signaling time would have been done anyway when the aborted transaction is finally rolled back, the only extra work is copying the relevant notifications to local memory. Regardless it might not be worthwhile since my patch for #1 seems to address the symptom that bit me. Matt Newell diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 3b71174..e89ece5 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -196,12 +196,24 @@ typedef struct QueuePosition #define QUEUE_POS_EQUAL(x,y) \ ((x).page == (y).page && (x).offset == (y).offset) +/* Returns 1 if x is larger than y, 0 if equal, else -1 */ +#define QUEUE_POS_COMPARE(x,y) \ + (((x).page > (y).page) ? 1 : \ + ((x).page < (y).page) ? -1 : \ + ((x).offset > (y).offset ? 1 : ((x).offset == (y).offset ? 0 : -1))) + /* choose logically smaller QueuePosition */ #define QUEUE_POS_MIN(x,y) \ (asyncQueuePagePrecedes((x).page, (y).page) ? (x) : \ (x).page != (y).page ? (y) : \ (x).offset < (y).offset ? (x) : (y)) +/* choose logically smaller QueuePosition */ +#define QUEUE_POS_MAX(x,y) \ + (asyncQueuePagePrecedes((x).page, (y).page) ? (y) : \ + (x).page != (y).page ? (x) : \ + (x).offset < (y).offset ? (y) : (x)) + /* * Struct describing a listening backend's status */ @@ -217,12 +229,13 @@ typedef struct QueueBackendStatus * The AsyncQueueControl structure is protected by the AsyncQueueLock. * * When holding the lock in SHARED mode, backends may only inspect their own - * entries as well as the head and tail pointers. Consequently we can allow a - * backend to update its own record while holding only SHARED lock (since no - * other backend will inspect it). + * entries as well as the head, tail, and firstUncommitted pointers. + * Consequently we can allow a backend to update its own record while holding + * only SHARED lock (since no other backend wi
[HACKERS] LISTEN denial of service with aborted transaction
This morning with our production database I began receiving reports of the database being "down". I checked the log and was surprised to see extremely long durations for a LISTEN that happens after each connection is made by our database library. This coincided with many(approx 600) new connections happening in a short time window due to render nodes automatically being turned on when the first job of the morning was submitted(idle nodes are turned off to save power). My initial hunch was that there was some code in postgres that resulted exponential execution time if enough listens on new connections happened at the same time. As I was trying to gather more information the listen times began to decrease and after about 20 minutes things were back to normal. A few hours later the symptoms returned but this time the listen was taking upwards of 15 minutes. I did some more reading and checked the pg_notify directory and found that there were 49 files. Having never checked that directory before I wasn't sure if that was normal. A short time later I noticed there was a process sitting idle with an aborted transaction. After killing the process things quickly returned to normal. After doing a search for "idle in transaction (aborted)" I came upon http://stackoverflow.com/questions/15036438/postgres-connection-leaks-idle-in-transaction-aborted While this is a potential solution for dealing with the problem it seems that the postgresql developers have decided to let connections stay in the "idle in transaction (aborted)" state for a reason, most likely under the assumption that it's relatively safe and only eats up the resources of a single connection. However it's easy to demonstrate that doing: listen "abc"; begin; select bad_column from non_existant_table; ...will eventually cause a denial of service situation if the DBA hasn't setup guards against connection sitting idle in an aborted transaction. Looking at the code in src/backend/commands/async.c I think there are a couple ways to eliminate this problem. 1. When a connection issues it's first LISTEN command, in Exec_ListenPreCommit QUEUE_BACKEND_POS(MyBackendId) = QUEUE_TAIL; this causes the connection to iterate through every notify queued in the slru, even though at that point I believe the connection can safely ignore any notifications from transactions that are already committed, and if I understand correctly notifications aren't put into the slru until precommit, so wouldn't it be safe to do: QUEUE_BACKEND_POS(MyBackendId) = QUEUE_HEAD; inside Async_Listen? If that's not safe, then could a new member be added to AsyncQueueControl that points to the first uncommitted QueuePosition (wouldn't have to be kept completely up to date). This would solve the problem of slow initial LISTEN in the face of a growing pg_notify queue. 2. Would it be possible when a backend is signaled to check if it is idle inside an aborted transaction, and if so process the notifications and put any that match what the backend is listening on into a local list. This would allow the slru to be cleaned up. In practice I think most notifications would either be disregarded or combined with a duplicate, so the list would most likely end up staying very small. If the backend local list does grow too large then the connection could be killed or handled in some other appropriate way. I am happy to attempt coming up with a patch if the ideas are deemed worthwhile. Thanks, Matt Newell -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical column ordering
> > Even if it does fit in memory I suspect memory bandwidth is more important > than clock cycles. http://people.freebsd.org/~lstewart/articles/cpumemory.pdf This paper is old but the ratios should still be pretty accurate. Main memory is 240 clock cycles away and L1d is only 3. If the experiments in this paper still hold true loading the 8K block into L1d is far more expensive than the CPU processing done once the block is in cache. When one adds in NUMA to the contention on this shared resource, its not that hard for a 40 core machine to starve for memory bandwidth, and for cores to sit idle waiting for main memory. Eliminating wasted space seems far more important even when everything could fit in memory already.
Re: [HACKERS] Exposing the stats snapshot timestamp to SQL
> > Yeah. The only use-case that's been suggested is detecting an > unresponsive stats collector, and the main timestamp should be plenty for > that. Lately, I've spent most of my time doing investigation into increasing qps. Turned out we've been able to triple our throughput by monitoring experiments at highly granular time steps (1 to 2 seconds). Effects that were invisible with 30 second polls of the stats were obvious with 2 second polls. The problem with doing highly granular snapshots is that the postgres counters are monotonically increasing, but only when stats are published. Currently you have no option except to divide by the delta of now() between the polling intervals. If you poll every 2 seconds the max error is about .5/2 or 25%. It makes reading those numbers a bit noisy. Using (snapshot_timestamp_new - snapshot_timestamp_old) as the denominator in that calculation should help to smooth out that noise and show a clearer picture. However, I'm happy with the committed version. Thanks Tom. - Matt K. On Thu, Feb 19, 2015 at 9:40 PM, Tom Lane wrote: > Matt Kelly writes: > > Attached is the fixed version. (hopefully with the right mime-type and > > wrong extension. Alas, gmail doesn't let you set mime-types; time to > find > > a new email client...) > > Committed with a couple of changes: > > * I changed the function name from pg_stat_snapshot_timestamp to > pg_stat_get_snapshot_timestamp, which seemed to me to be the style > in general use in the stats stuff for inquiry functions. > > * The function should be marked stable not volatile, since its value > doesn't change any faster than all the other stats inquiry functions. > > regards, tom lane >
Re: [HACKERS] Exposing the stats snapshot timestamp to SQL
​Actually, I just did one more code review of myself, and somehow missed
that I submitted the version with the wrong oid. The oid used in the first
version is wrong (1) and was from before I read the docs on properly
picking one.
Attached is the fixed version. (hopefully with the right mime-type and
wrong extension. Alas, gmail doesn't let you set mime-types; time to find
a new email client...)
- Matt K.
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
***
*** 1710,1715 postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34
0:00 postgres: ser
--- 1710,1723
+
pg_stat_snapshot_timestamp()pg_stat_snapshot_timestamp
+ timestamp with time zone
+
+Returns the timestamp of the current statistics snapshot
+
+
+
+
pg_stat_clear_snapshot()pg_stat_clear_snapshot
void
*** a/src/backend/utils/adt/pgstatfuncs.c
--- b/src/backend/utils/adt/pgstatfuncs.c
***
*** 115,120 extern Datum pg_stat_get_xact_function_calls(PG_FUNCTION_ARGS);
--- 115,122
extern Datum pg_stat_get_xact_function_total_time(PG_FUNCTION_ARGS);
extern Datum pg_stat_get_xact_function_self_time(PG_FUNCTION_ARGS);
+ extern Datum pg_stat_snapshot_timestamp(PG_FUNCTION_ARGS);
+
extern Datum pg_stat_clear_snapshot(PG_FUNCTION_ARGS);
extern Datum pg_stat_reset(PG_FUNCTION_ARGS);
extern Datum pg_stat_reset_shared(PG_FUNCTION_ARGS);
***
*** 1681,1686 pg_stat_get_xact_function_self_time(PG_FUNCTION_ARGS)
--- 1683,1694
PG_RETURN_FLOAT8(INSTR_TIME_GET_MILLISEC(funcentry->f_counts.f_self_time));
}
+ /* Get the timestamp of the current statistics snapshot */
+ Datum
+ pg_stat_snapshot_timestamp(PG_FUNCTION_ARGS)
+ {
+ PG_RETURN_TIMESTAMPTZ(pgstat_fetch_global()->stats_timestamp);
+ }
/* Discard the active statistics snapshot */
Datum
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 2852,2857 DESCR("statistics: total execution time of function in
current transaction, in m
--- 2852,2860
DATA(insert OID = 3048 ( pg_stat_get_xact_function_self_time PGNSP PGUID 12
1 0 0 0 f f f f t f v 1 0 701 "26" _null_ _null_ _null_ _null_
pg_stat_get_xact_function_self_time _null_ _null_ _null_ ));
DESCR("statistics: self execution time of function in current transaction, in
msec");
+ DATA(insert OID = 3788 ( pg_stat_snapshot_timestamp PGNSP PGUID 12 1 0 0 0
f f f f t f v 0 0 1184 "" _null_ _null_ _null_ _null_
pg_stat_snapshot_timestamp _null_ _null_ _null_ ));
+ DESCR("statistics: timestamp of the current statistics snapshot");
+
DATA(insert OID = 2230 ( pg_stat_clear_snapshot PGNSP PGUID 12
1 0 0 0 f f f f f f v 0 0 2278 "" _null_ _null_ _null_ _null_
pg_stat_clear_snapshot _null_ _null_ _null_ ));
DESCR("statistics: discard current transaction's statistics snapshot");
DATA(insert OID = 2274 ( pg_stat_reset
PGNSP PGUID 12 1 0 0 0 f f f f f f v 0 0 2278 "" _null_ _null_ _null_ _null_
pg_stat_reset _null_ _null_ _null_ ));
*** a/src/test/regress/expected/stats.out
--- b/src/test/regress/expected/stats.out
***
*** 28,34 SELECT pg_sleep_for('2 seconds');
CREATE TEMP TABLE prevstats AS
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
!(b.idx_blks_read + b.idx_blks_hit) AS idx_blks
FROM pg_catalog.pg_stat_user_tables AS t,
pg_catalog.pg_statio_user_tables AS b
WHERE t.relname='tenk2' AND b.relname='tenk2';
--- 28,35
CREATE TEMP TABLE prevstats AS
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
!(b.idx_blks_read + b.idx_blks_hit) AS idx_blks,
!pg_stat_snapshot_timestamp() as snap_ts
FROM pg_catalog.pg_stat_user_tables AS t,
pg_catalog.pg_statio_user_tables AS b
WHERE t.relname='tenk2' AND b.relname='tenk2';
***
*** 111,114 SELECT st.heap_blks_read + st.heap_blks_hit >= pr.heap_blks +
cl.relpages,
--- 112,122
t| t
(1 row)
+ SELECT pr.snap_ts < pg_stat_snapshot_timestamp() as snapshot_newer
+ FROM prevstats AS pr;
+ snapshot_newer
+
+ t
+ (1 row)
+
-- End of Stats Test
*** a/src/test/regress/sql/stats.sql
--- b/src/test/regress/sql/stats.sql
***
*** 22,28 SELECT pg_sleep_for('2 seconds');
CREATE TEMP TABLE prevstats AS
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
!(b.idx_blks_read + b.idx_blks_hit) AS idx_blks
FROM pg_catalog.pg_stat_user_tables AS t,
Re: [HACKERS] Exposing the stats snapshot timestamp to SQL
On Thu, Jan 29, 2015 at 8:42 PM, Tom Lane wrote: > Matt Kelly writes: > > Jim, I'm not sure I understand what you mean? This new function follows > > the same conventions as everything else in the file. TimestampTz is > just a > > typedef for int64. > > ... or double. Have you checked that the code behaves properly with > --disable-integer-timestamps? > > regards, tom lane > Well, yes int or double. I should have been more clear about that. Its a good point though that I should run the server with disable for completeness. I presume you meant --disable-integer-datetimes. I just ran that test case now, all set. For my own edification, was there really any risk of something so trivial breaking due to that setting, or was it just for completeness? (which is a perfectly valid reason) Thanks, - Matt K.
Re: [HACKERS] Exposing the stats snapshot timestamp to SQL
This is now: https://commitfest.postgresql.org/4/128/
On Thu, Jan 29, 2015 at 7:01 PM, Matt Kelly wrote:
> Robert, I'll add it to the commitfest.
>
> Jim, I'm not sure I understand what you mean? This new function follows
> the same conventions as everything else in the file. TimestampTz is just a
> typedef for int64. Functions like pg_stat_get_buf_alloc follow the exact
> same pattern on the int64 fields of the global stats struct.
>
> - Matt K.
>
> On Thu, Jan 29, 2015 at 6:49 PM, Jim Nasby
> wrote:
>
>> On 1/28/15 11:18 PM, Matt Kelly wrote:
>>
>>> In a previous thread Tom Lane said:
>>>
>>> (I'm also wondering if it'd make sense to expose the stats timestamp
>>> as a callable function, so that the case could be dealt with
>>> programmatically as well. But that's future-feature territory.)
>>>
>>> (http://www.postgresql.org/message-id/27251.1421684...@sss.pgh.pa.us)
>>>
>>> It seemed the appropriate scope for my first submission, and that
>>> feature has been on my wish list for a while, so I thought I'd grab it.
>>>
>>
>> I've reviewed the patch (though haven't tested it myself) and it looks
>> good. The only thing I'm not sure of is this:
>>
>> + /* Get the timestamp of the current statistics snapshot */
>> + Datum
>> + pg_stat_snapshot_timestamp(PG_FUNCTION_ARGS)
>> + {
>> + PG_RETURN_TIMESTAMPTZ(pgstat_fetch_global()->stats_timestamp);
>> + }
>>
>> Is the community OK with referencing stats_timestamp that way?
>> --
>> Jim Nasby, Data Architect, Blue Treble Consulting
>> Data in Trouble? Get it in Treble! http://BlueTreble.com
>>
>
>
Re: [HACKERS] Exposing the stats snapshot timestamp to SQL
Robert, I'll add it to the commitfest.
Jim, I'm not sure I understand what you mean? This new function follows
the same conventions as everything else in the file. TimestampTz is just a
typedef for int64. Functions like pg_stat_get_buf_alloc follow the exact
same pattern on the int64 fields of the global stats struct.
- Matt K.
On Thu, Jan 29, 2015 at 6:49 PM, Jim Nasby wrote:
> On 1/28/15 11:18 PM, Matt Kelly wrote:
>
>> In a previous thread Tom Lane said:
>>
>> (I'm also wondering if it'd make sense to expose the stats timestamp
>> as a callable function, so that the case could be dealt with
>> programmatically as well. But that's future-feature territory.)
>>
>> (http://www.postgresql.org/message-id/27251.1421684...@sss.pgh.pa.us)
>>
>> It seemed the appropriate scope for my first submission, and that feature
>> has been on my wish list for a while, so I thought I'd grab it.
>>
>
> I've reviewed the patch (though haven't tested it myself) and it looks
> good. The only thing I'm not sure of is this:
>
> + /* Get the timestamp of the current statistics snapshot */
> + Datum
> + pg_stat_snapshot_timestamp(PG_FUNCTION_ARGS)
> + {
> + PG_RETURN_TIMESTAMPTZ(pgstat_fetch_global()->stats_timestamp);
> + }
>
> Is the community OK with referencing stats_timestamp that way?
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>
[HACKERS] Exposing the stats snapshot timestamp to SQL
In a previous thread Tom Lane said: (I'm also wondering if it'd make sense to expose the stats timestamp > as a callable function, so that the case could be dealt with > programmatically as well. But that's future-feature territory.) (http://www.postgresql.org/message-id/27251.1421684...@sss.pgh.pa.us) It seemed the appropriate scope for my first submission, and that feature has been on my wish list for a while, so I thought I'd grab it. Main purpose of this patch is to expose the timestamp of the current stats snapshot so that it can be leveraged by monitoring code. The obvious case is alerting if the stats collector becomes unresponsive. The common use case is to smooth out graphs which are built from high frequency monitoring of the stats collector. The timestamp is already available as part of PgStat_GlobalStats. This patch is just the boilerplate (+docs & tests) needed to expose that value to SQL. It shouldn't impact anything else in the server. I'm not particularly attached to the function name, but I didn't have a better idea. The patch should apply cleanly to master. - Matt K pg_stat_snapshot_timestamp_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] Better way of dealing with pgstat wait timeout during buildfarm runs?
> > Sure, but nobody who is not a developer is going to care about that. > A typical user who sees "pgstat wait timeout", or doesn't, isn't going > to be able to make anything at all out of that. As a user, I wholeheartedly disagree. That warning helped me massively in diagnosing an unhealthy database server in the past at TripAdvisor (i.e. high end server class box, not a raspberry pie). I have realtime monitoring that looks at pg_stat_database at regular intervals particularly for the velocity of change of xact_commit and xact_rollback columns, similar to how check_postgres does it. https://github.com/bucardo/check_postgres/blob/master/check_postgres.pl#L4234 When one of those servers was unhealthy, it stopped reporting statistics for 30 seconds+ at a time. My dashboard which polled far more frequently than that indicated the server was normally processing 0 tps with intermittent spikes. I went directly onto the server and sampled pg_stat_database. That warning was the only thing that directly indicated that the statistics collector was not to be trusted. It obviously was a victim of what was going on in the server, but its pretty important to know when your methods for measuring server health are lying to you. The spiky TPS at first glance appears like some sort of live lock, not just that the server is overloaded. Now, I know: 0 change in stats = collector broken. Rereading the docks, Also, the collector itself emits a new report at most once per > PGSTAT_STAT_INTERVAL milliseconds (500 ms unless altered while building > the server). Without context this merely reads: "We sleep for 500ms, plus the time to write the file, plus whenever the OS decides to enforce the timer interrupt... so like 550-650ms." It doesn't read, "When server is unhealthy, but _still_ serving queries, the stats collector might not be able to keep up and will just stop reporting stats all together." I think the warning is incredibly valuable. Along those lines I'd also love to see a pg_stat_snapshot_timestamp() for monitoring code to use to determine if its using a stale snapshot, as well as helping to smooth graphs of the statistics that are based on highly granular snapshotting. - Matt Kelly
Re: [HACKERS] Async execution of postgres_fdw.
I'm trying to compare v5 and v6 in my laptop right now. Apparently my laptop is quite a bit faster than your machine because the tests complete in roughly 3.3 seconds. I added more data and didn't see anything other than noise. (Then again the queries were dominated by the disk sort so I should retry with larger work_mem). I'll try it again when I have more time to play with it. I suspect the benefits would be more clear over a network. Larger than default work_mem yes, but I think one of the prime use case for the fdw is for more warehouse style situations (PostgresXL style use cases). In those cases, work_mem might reasonably be set to 1GB. Then even if you have 10KB rows you can fetch a million rows and still be using less than work_mem. A simpler change would be to vary it with respect to work_mem. Half baked idea: I know its the wrong time in the execution phase, but if you are using remote estimates for cost there should also be a row width estimate which I believe is based from pg_statistic and its mean column width. Its actually a pity that there is no way to set fetch sizes based on "give me as many tuples as will fit in less than x amount of memory". Because that is almost always exactly what you want. Even when writing application code, I've never actually wanted precisely 10,000 rows; I've always wanted "a reasonable size chunk that could fit into memory" and then backed my way into how many rows I wanted. If we were to extend FETCH to support syntax like: FETCH FORWARD '10MB' FROM ...; then we would eliminate the need estimate the value on the fly. The async stuff, however, is a huge improvement over the last time I played with the fdw. The two active postgres processes were easily consuming a core and half of CPU. I think its not worth tying these two things together. Its probably worth it to make these two separate discussions and separate patches. - Matt Kelly *Just sanity checking myself: Shutting down the server, applying the different patch, 'make clean install' in postgres_fdw, and then restarting the server should obviously be sufficient to make sure its running the new code because that is all linked at runtime, right?
Re: [HACKERS] Async execution of postgres_fdw.
I think its telling that varying the fetch size doubled the performance, even on localhost. If you were to repeat this test across a network, the performance difference would be far more drastic. I understand the desire to keep the fetch size small by default, but I think your results demonstrate how important the value is. At the very least, it is worth reconsidering this "arbitrary" value. However, I think the real solution is to make this configurable. It probably should be a new option on the foreign server or table, but an argument could be made for it to be global across the server just like work_mem. Obviously, this shouldn't block your current patch but its worth revisiting. - Matt Kelly
Re: [HACKERS] libpq pipelining
On Friday, December 05, 2014 12:22:38 PM Heikki Linnakangas wrote: > Oh, that's what the PQgetLastQuery/PQgetNextQuery functions work! I > didn't understand that before. I'd suggest renaming them to something > like PQgetSentQuery() and PQgetResultQuery(). The first/last/next names > made me think that they're used to iterate a list of queries, but in > fact they're supposed to be used at very different stages. > > - Heikki Okay, I have renamed them with your suggestions, and added PQsetPipelining/PQgetPipelining, defaulting to pipelining off. There should be no behavior change unless pipelining is enabled. Documentation should be mostly complete except the possible addition of an example and maybe a general pipelining overview paragraph. I have implemented async query support (that takes advantage of pipelining) in Qt, along with a couple test cases. This led to me discovering a bug with my last patch where a PGquery object could be reused twice in a row. I have fixed that. I contemplated not reusing the PGquery objects at all, but that wouldn't solve the problem because it's very possible that malloc will return a recent free of the same size anyway. Making the guarantee that a PGquery won't be reused twice in a row should be sufficient, and the only alternative is to add a unique id, but that will add further complexity that I don't think is warranted. Feedback is very welcome and appreciated. Thanks, Matt Newell diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index d829a4b..4e0431e 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -3947,9 +3947,14 @@ int PQsendQuery(PGconn *conn, const char *command); After successfully calling PQsendQuery, call PQgetResult one or more times to obtain the - results. PQsendQuery cannot be called again - (on the same connection) until PQgetResult - has returned a null pointer, indicating that the command is done. + results. If pipelining is enabled PQsendQuery + may be called multiple times before reading the results. See + PQsetPipelining and PQisPipelining. + Call PQgetSentQuery to get a PGquery + which can be used to identify which results obtained from + PQgetResult belong to each pipelined query. + If only one query is dispatched at a time, you can call PQgetResult + until a NULL value is returned to indicate the end of the query. @@ -4133,8 +4138,8 @@ PGresult *PQgetResult(PGconn *conn); PQgetResult must be called repeatedly until - it returns a null pointer, indicating that the command is done. - (If called when no command is active, + it returns a null pointer, indicating that all dispatched commands + are done. (If called when no command is active, PQgetResult will just return a null pointer at once.) Each non-null result from PQgetResult should be processed using the @@ -4144,14 +4149,17 @@ PGresult *PQgetResult(PGconn *conn); PQgetResult will block only if a command is active and the necessary response data has not yet been read by PQconsumeInput. + If query pipelining is being used, PQgetResultQuery + can be called after PQgetResult to match the result to the query. Even when PQresultStatus indicates a fatal -error, PQgetResult should be called until it -returns a null pointer, to allow libpq to -process the error information completely. +error, PQgetResult should be called until the +query has no more results (null pointer return if not using query +pipelining, otherwise see PQgetResultQuery), +to allow libpq to process the error information completely. @@ -4385,6 +4393,158 @@ int PQflush(PGconn *conn); read-ready and then read the response as described above. + + + +PQsetPipelining + + PQsetPipelining + + + + + + Enables or disables query pipelining. + +int PQsetPipelining(PGconn *conn, int arg); + + + + + Enables pipelining for the connectino if arg is 1, or disables it + if arg is 0. When pipelining is enabled multiple async queries can + be sent before processing the results of the first. If pipelining + is disabled an error will be raised an async query is attempted + while another is active. + + + + + + +PQisPipelining + + PQisPipelining + + + + + + Returns the pipelining status of the connection + +int PQisPipelining(PGconn *conn); + + + + + Returns 1 if pipelining is enabled, or 0 if pipeling is disabled. + Query pipelining is disabled unless enabled with a call to + PQsetPipelining. + + + + + + +PQgetQueryCommand + + PQgetQueryCommand +
Re: [HACKERS] libpq pipelining
>
> > The explanation of PQgetFirstQuery makes it sound pretty hard to match
> > up the result with the query. You have to pay attention to PQisBusy.
>
> PQgetFirstQuery should also be valid after
> calling PQgetResult and then you don't have to worry about PQisBusy, so I
> should probably change the documentation to indicate that is the preferred
> usage, or maybe make that the only guaranteed usage, and say the results
> are undefined if you call it before calling PQgetResult. That usage also
> makes it consistent with PQgetLastQuery being called immediately after
> PQsendQuery.
>
I changed my second example to call PQgetFirstQuery after PQgetResult instead
of before, and that removes the need to call PQconsumeInput and PQisBusy when
you don't mind blocking. It makes the example super simple:
PQsendQuery(conn, "INSERT INTO test(id) VALUES (DEFAULT),(DEFAULT)
RETURNING id");
query1 = PQgetLastQuery(conn);
/* Duplicate primary key error */
PQsendQuery(conn, "UPDATE test SET id=2 WHERE id=1");
query2 = PQgetLastQuery(conn);
PQsendQuery(conn, "SELECT * FROM test");
query3 = PQgetLastQuery(conn);
while( (result = PQgetResult(conn)) != NULL )
{
curQuery = PQgetFirstQuery(conn);
if (curQuery == query1)
checkResult(conn,result,curQuery,PGRES_TUPLES_OK);
if (curQuery == query2)
checkResult(conn,result,curQuery,PGRES_FATAL_ERROR);
if (curQuery == query3)
checkResult(conn,result,curQuery,PGRES_TUPLES_OK);
}
Note that the curQuery == queryX check will work no matter how many results a
query produces.
Matt Newell
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] libpq pipelining
tainly be used that way. My first test case does continuous pipelining and it provides a huge throughput gain when there's any latency in the connection. I can envision a lot of uses for the continuous approach. > >> Consideration of implicit transactions (autocommit), the whole pipeline > >> being one transaction, or multiple transactions is needed. > > > > The more I think about this the more confident I am that no extra work is > > needed. > > > > Unless we start doing some preliminary processing of the query inside of > > libpq, our hands are tied wrt sending a sync at the end of each query. > > The > > reason for this is that we rely on the ReadyForQuery message to indicate > > the end of a query, so without the sync there is no way to tell if the > > next result is from another statement in the current query, or the first > > statement in the next query. > > > > I also don't see a reason to need multiple queries without a sync > > statement. If the user wants all queries to succeed or fail together it > > should be no problem to start the pipeline with begin and complete it > > commit. But I may be missing some detail... > > True. It makes me a bit uneasy, though, to not be sure that the whole > batch is committed or rolled back as one unit. There are many ways the > user can shoot himself in the foot with that. Error handling would be a > lot simpler if you would only send one Sync for the whole batch. Tom > explained it better on this recent thread: > http://www.postgresql.org/message-id/32086.1415063...@sss.pgh.pa.us. > I've read tom's email and every other one i could find related to pipelining and I simply don't see the problem. What's the advantage of being able to pipeline multiple queries without sync and without an explicit transaction, vs an explicit transaction with a sync per query? The usage I have in mind for pipelining needs each query to succeed or fail independently of any query before or after it, unless the caller explicitly uses a transaction. > Another thought is that for many applications, it would actually be OK > to not know which query each result belongs to. For example, if you > execute a bunch of inserts, you often just want to get back the total > number of inserted, or maybe not even that. Or if you execute a "CREATE > TEMPORARY TABLE ... ON COMMIT DROP", followed by some insertions to it, > some more data manipulations, and finally a SELECT to get the results > back. All you want is the last result set. Easy, grab the PGquery from only the last select, call PQgetResult until you get a matching result, PQclear the rest. We could provide a function to do just that as a convenience to the user, if it's going to be a common use-case. It would be no different than how PQexec processes and discards results. Matt -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] libpq pipelining
On Thursday, December 04, 2014 04:30:27 PM Claudio Freire wrote: > On Thu, Dec 4, 2014 at 4:11 PM, Matt Newell wrote: > > With the API i am proposing, only 2 new functions (PQgetFirstQuery, > > PQgetLastQuery) are required to be able to match each result to the query > > that caused it. Another function, PQgetNextQuery allows iterating > > through the pending queries, and PQgetQueryCommand permits getting the > > original query text. > > > > Adding the ability to set a user supplied pointer on the PGquery struct > > might make it much easier for some frameworks, and other users might want > > a callback, but I don't think either are required. > > With a pointer on PGquery you wouldn't need any of the above. Who > whants the query text sets it as a pointer, who wants some other > struct sets it as a pointer. > libpq already stores the (current) query text as it's used in some error cases, so that's not really optional without breaking backwards compatibility. Adding another pointer for the user to optional utilize should be no big deal though if everyone agrees it's a good thing. > You would only need to be careful about the lifetime of the pointed > struct, but that onus is on the application I'd say. The API only > needs to provide some guarantees about how long or short it holds onto > that pointer. Agreed. > > I'm thinking this would be somewhat necessary for a python wrapper, > like psycopg2 (the wrapper could build a dictionary based on query > text, but there's no guarantee that query text will be unique so it'd > be very tricky). While it might make some things simpler, i really don't think it absolutely necessary since the wrapper can maintain a queue that corresponds to libpq's internal queue of PGquery's. ie, each time you call a PQsendQuery* function you push your required state, and each time the return value of PQgetFirstQuery changes you pop from the queue. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] libpq pipelining
On Thursday, December 04, 2014 10:30:46 PM Craig Ringer wrote:
> On 12/04/2014 05:08 PM, Heikki Linnakangas wrote:
> > A good API is crucial for this. It should make it easy to write an
> > application that does pipelining, and to handle all the error conditions
> > in a predictable way. I'd suggest that you write the documentation
> > first, before writing any code, so that we can discuss the API. It
> > doesn't have to be in SGML format yet, a plain-text description of the
> > API will do.
>
> I strongly agree.
>
First pass at the documentation changes attached, along with a new example
that demonstrates pipelining 3 queries, with the middle one resulting in a
PGRES_FATAL_ERROR response.
With the API i am proposing, only 2 new functions (PQgetFirstQuery,
PQgetLastQuery) are required to be able to match each result to the query that
caused it. Another function, PQgetNextQuery allows iterating through the
pending queries, and PQgetQueryCommand permits getting the original query
text.
Adding the ability to set a user supplied pointer on the PGquery struct might
make it much easier for some frameworks, and other users might want a
callback, but I don't think either are required.
> Applications need to be able to reliably predict what will happen if
> there's an error in the middle of a pipeline.
>
Yes, the API i am proposing makes it easy to get results for each submitted
query independently of the success or failure of previous queries in the
pipeline.
> Consideration of implicit transactions (autocommit), the whole pipeline
> being one transaction, or multiple transactions is needed.
The more I think about this the more confident I am that no extra work is
needed.
Unless we start doing some preliminary processing of the query inside of
libpq, our hands are tied wrt sending a sync at the end of each query. The
reason for this is that we rely on the ReadyForQuery message to indicate the
end of a query, so without the sync there is no way to tell if the next result
is from another statement in the current query, or the first statement in the
next query.
I also don't see a reason to need multiple queries without a sync statement.
If the user wants all queries to succeed or fail together it should be no
problem to start the pipeline with begin and complete it commit. But I may be
missing some detail...
>
> Apps need to be able to wait for the result of a query partway through a
> pipeline, e.g. scheduling four queries, then waiting for the result of
> the 2nd.
>
Right. With the api i am proposing the user does have to process each result
until it gets to the one it wants, but it's no problem doing that. It would
also be trivial to add a function
PGresult * PQgetNextQueryResult(PQquery *query);
that discards all results from previous queries. Very similar to how a PQexec
disregards all results from previous async queries.
It would also be possible to queue the results and be able to retrieve them
out of order, but I think that add unnecessary complexity and might also make
it easy for users to never retrieve and free some results.
> There are probably plenty of other wrinkly bits to think about.
Yup, I'm sure i'm still missing some significant things at this point...
Matt Newell
/*
* src/test/examples/testlibpqpipeline2.c
*
*
* testlibpqpipeline.c
* this test program tests query pipelining. It shows how to issue multiple
* pipelined queries, and identify from which query a result originated. It
* also demonstrates how failure of one query does not impact subsequent queries
* when they are not part of the same transaction.
*
*
*/
#include
#include
#include
#include "libpq-fe.h"
static void checkResult(PGconn *conn, PGresult *result, PGquery *query, int expectedResultStatus)
{
if (PQresultStatus(result) != expectedResultStatus)
{
printf( "Got unexpected result status '%s', expected '%s'\nQuery:%s\n",
PQresStatus(PQresultStatus(result)), PQresStatus(expectedResultStatus),
PQgetQueryCommand(query));
PQclear(result);
PQclear(PQexec(conn,"DROP TABLE test"));
PQfinish(conn);
exit(1);
}
PQclear(result);
}
int
main(int argc, char **argv)
{
PGconn * conn;
PGquery * query1;
PGquery * query2;
PGquery * query3;
PGquery * curQuery;
PGresult * result;
conn = NULL;
query1 = query2 = query3 = curQuery = NULL;
result = NULL;
/* make a connection to the database */
conn = PQsetdb(NULL, NULL, NULL, NULL, NULL);
/* check to see that the backend connection was successfully made */
if (PQstatus(conn) != CONNECTION_OK)
{
fprintf(stderr, "Connection to database failed: %s",
PQerrorMessage(conn));
exit(1);
}
checkResult(conn,PQexec(conn,"DROP TABLE IF EXISTS test"),NULL,PGRES_COMMAND_OK);
checkResult(conn,PQexec(
[HACKERS] libpq pipelining
Hi,
The recent discussion about pipelining in the jodbc driver prompted me to look
at what it would take for libpq.
I have a proof of concept patch working. The results are even more promising
than I expected.
While it's true that many applications and frameworks won't easily benefit, it
amazes me that this hasn't been explored before.
I developed a simple test application that creates a table with a single auto
increment primary key column, then runs a 4 simple queries x times each:
"INSERT INTO test() VALUES ()"
"SELECT * FROM test LIMIT 1"
"SELECT * FROM test"
"DELETE FROM test"
The parameters to testPipelinedSeries are (number of times to execute each
query, maximum number of queued queries).
Results against local server:
testPipelinedSeries(10,1) took 0.020884
testPipelinedSeries(10,3) took 0.020630, speedup 1.01
testPipelinedSeries(10,10) took 0.006265, speedup 3.33
testPipelinedSeries(100,1) took 0.042731
testPipelinedSeries(100,3) took 0.043035, speedup 0.99
testPipelinedSeries(100,10) took 0.037222, speedup 1.15
testPipelinedSeries(100,25) took 0.031223, speedup 1.37
testPipelinedSeries(100,50) took 0.032482, speedup 1.32
testPipelinedSeries(100,100) took 0.031356, speedup 1.36
Results against remote server through ssh tunnel(30-40ms rtt):
testPipelinedSeries(10,1) took 3.2461736
testPipelinedSeries(10,3) took 1.1008443, speedup 2.44
testPipelinedSeries(10,10) took 0.342399, speedup 7.19
testPipelinedSeries(100,1) took 26.25882588
testPipelinedSeries(100,3) took 8.8509234, speedup 3.04
testPipelinedSeries(100,10) took 3.2866285, speedup 9.03
testPipelinedSeries(100,25) took 2.1472847, speedup 17.57
testPipelinedSeries(100,50) took 1.957510, speedup 27.03
testPipelinedSeries(100,100) took 0.690682, speedup 37.47
I plan to write documentation, add regression testing, and do general cleanup
before asking for feedback on the patch itself. Any suggestions about
performance testing or api design would be nice. I haven't played with
changing the sync logic yet, but I'm guessing that an api to allow manual sync
instead of a sync per PQsendQuery will be needed. That could make things
tricky though with multi-statement queries, because currently the only way to
detect when results change from one query to the next are a ReadyForQuery
message.
Matt Newell
/*
* src/test/examples/testlibpqpipeline.c
*
*
* testlibpqpipeline.c
* this test program test query pipelining and it's performance impact
*
*
*/
#include
#include
#include
#include "libpq-fe.h"
// If defined we won't issue more sql commands if the socket's
// write buffer is full
//#define MIN_LOCAL_Q
//#define PRINT_QUERY_PROGRESS
static int testPipelined( PGconn * conn, int totalQueries, int totalQueued, const char * sql );
static int testPipelinedSeries( PGconn * conn, int totalQueries, int totalQueued, int baseline_usecs );
int
testPipelined( PGconn * conn, int totalQueries, int totalQueued, const char * sql )
{
int nQueriesQueued;
int nQueriesTotal;
PGresult * result;
PGquery * firstQuery;
PGquery * curQuery;
nQueriesQueued = nQueriesTotal = 0;
result = NULL;
firstQuery = curQuery = NULL;
while( nQueriesQueued > 0 || nQueriesTotal < totalQueries ) {
if( PQconsumeInput(conn) == 0 ) {
printf( "PQconsumeInput ERROR: %s\n", PQerrorMessage(conn) );
return 1;
}
do {
curQuery = PQgetFirstQuery(conn);
/* firstQuery is finished */
if( firstQuery != curQuery )
{
//printf( "%p done, curQuery=%p\n", firstQuery, curQuery );
#ifdef PRINT_QUERY_PROGRESS
printf("-");
#endif
firstQuery = curQuery;
nQueriesQueued--;
}
/* Break if no queries are ready */
if( !firstQuery || PQisBusy(conn) )
break;
if( (result = PQgetResult(conn)) != 0 )
PQclear(result);
}
while(1);
if( nQueriesTotal < totalQueries && nQueriesQueued < totalQueued ) {
#ifdef MIN_LOCAL_Q
int flushResult = PQflush(conn);
if( flushResult == -1 ) {
printf( "PQflush ERROR: %s\n", PQerrorMessage(conn) );
return 1;
} else if ( flushResult == 1 )
continue;
#endif
PQsendQuery(conn,sql);
if( firstQuery == NULL )
firstQuery = PQgetFirstQuery(conn);
nQueriesTotal++;
nQueriesQueued++;
#ifdef PRINT_QUERY_PROGRESS
printf( "+" );
#endif
}
}
#ifdef PRINT_QUERY_PROGRESS
printf( "\n" );
#endif
return 0;
}
int testPipelinedSeries( PGconn * conn, int totalQueries, int totalQueued, int baseline_usecs )
{
int result;
struct timeval tv1, tv2;
int secs, usecs;
gettimeofday(&tv1,NULL);
#define TEST_P(q) \
if( (result = testPipelined(conn,totalQueries,totalQueued,q)) != 0 ) \
return result;
TEST_P("INSERT INTO test() VALUES ()");
TEST_P("SELECT * FROM test LIMIT 1");
TEST_P("SELECT * FRO
Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD
On Jun 24, 2014, at 9:42 AM, Tom Lane wrote: > I think this means we can write off VAX on NetBSD/OpenBSD as a viable > platform for Postgres :-(. I'm sad to hear it, but certainly have > not got the cycles personally to prevent it. Why? NetBSD/vax has supported shared libraries for a long long time. -- 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: OT: OFF TOPIC: returning multiple result sets from a stored procedure
>I honestly do not mean any offence, just out of curiosity. >If you guys care about money and time why would you spend the best years of >your life basically copying commercial products for free? 1) For the same reasons commercial vendors build competing products: different tools in the same category fit different scenarios 2) The right-tool-for-the-job concept: Sometimes a commercial solution is the right choice, sometime an open-source solution is. Budgets, applicability, and local expertise all should factor into platform selection decision. 3) Because sometimes the output of a platform is more important than the platform itself: If your role is to work with data, not to produce software, it can in the end be more efficient in the end to have a platform you can modify as needed.
Re: [HACKERS] parse tree to XML format
--- On Mon, 12/28/09, Andres Freund wrote: > From: Andres Freund > Subject: Re: [HACKERS] parse tree to XML format > To: pgsql-hackers@postgresql.org > Cc: "Robert Haas" , "matt" > Date: Monday, December 28, 2009, 7:39 PM > On Tuesday 29 December 2009 01:35:25 > Robert Haas wrote: > > On Mon, Dec 28, 2009 at 7:32 PM, Andres Freund > wrote: > > > On Monday 28 December 2009 22:30:44 matt wrote: > > >> Is there some way to export the postgresql > query parse tree in XML > > >> format? I can not locate the API/Tool etc to > do that... > > > > > > Thats more of a -general question. > > > > > > There is no such possibility in 8.4 - the not yet > released 8.5 contains > > > such a possibility. > > > > Well, you can export the plan as XML using EXPLAIN > (FORMAT XML), but > > that's not the same thing as the query parse-tree. > Uh. Err. Sorry. > > You can play around with "debug_print_parse" but thats many > things but > definitely not xml. > > Matt, what are you trying to achieve? We are trying to gather statistics about our queries and get automatic suggestions for what indexes to utilize ...its easier to figure that on queries that are in some format else we have to effectively parse the queries ourself or hack the postgresql parser...which we dont want to do... Did you mention that the 8.5 code has such a functionality? i would like to download the code and play with it a bit, any pointers what i need to do to get the XML? regards Matt > > Andres > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] parse tree to XML format
Is there some way to export the postgresql query parse tree in XML format? I can not locate the API/Tool etc to do that... thanks -Matt -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any tutorial or FAQ on building an extension?
On Aug 11, 1:11Â pm, j...@agliodbs.com (Josh Berkus) wrote: > > Is there an easier way of going about this other than replacing the > > postmaster / postgres components? > > I'd start with creating my own extended version to psql (the client > library), I suppose. Â But since I don't really know what kind of > "transformations" you have in mind, any advice is going to be purely > speculative. > Thanks for the response Josh. I'm not sure that psql is the right thing for me to do though, since I want to build a back-end component that takes the place of the existing postmaster. Very possible I misunderstood you though. To clarify, essentially what I want to do is this: Client [ psql | JDBC driver | pgAdmin | etc. ] issues a Query [ "Select * from sales" ] | | \/ My new component intercepts this, and decides if it wants to do something | | \/ If it does not, it simply passes this on to the real PostgreSQL server running somewhere | | \/ If it does, it passes the request over to my new server (via sockets), does its work, and pass back the results | | \/ The client gets the results back, either from PostgreSQL or from my new server, and goes about its way. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Any tutorial or FAQ on building an extension?
Hello Group, I'd like to build an extension to PostgreSQL. It will intercept queries and perform some transformations on the query and on the data returned, given some business rules that the users have specified. What's the best way to do this? It seems like if I model the pgpool- II architecture that would be a good start. That way existing clients don't really know that they are talking to my application instead of a real PostgreSQL postmaster. My app won't be written in C but I suppose I could start with this approach. Is there an easier way of going about this other than replacing the postmaster / postgres components? Thanks for any pointers, Matt -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] "anyelement2" pseudotype
> > adding an "anyelement2" pseudotype ... The context was a > > compatibility SQL function to support Oracle's DECODE function. > > The reason it's not in there already is we didn't seem to have quite > enough use-case to justify it. Do you have more? No. Even this case, for me, is more an expedient than a necessity. I could just rewrite my Oracle code to use CASE, but I've a lot of code to convert, and the transformation is a bit error prone. I'm also looking at a scripted code edit to rewrite the Oracle stuff, and comparing this to the cost a PG compatibility function. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[HACKERS] "anyelement2" pseudotype
A few months ago at http://archives.postgresql.org/pgsql-general/2006-11/msg01770.php the notion of adding an "anyelement2" pseudotype was discussed. The context was a compatibility SQL function to support Oracle's DECODE function. Assuming this new pseudotype has not been added yet, I'm ready to look into doing this myself, and I'd like a bit of shove in the right direction. ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] 8.2.0 Tarball vs. REL8_2_0 vs. REL8_2_STABLE
> > The [pgcluster-1.7.0rc1-patch] patch applies to the 8.2.0 tarball ... > > However, the patch will not apply to cvs branch REL8_2_0. > > I've been told that the pgcluster patch patches some generated files > (parse.h and other apparently). Yes, I could not at first apply to REL8_2_0 because the patch file wanted to patch src/backend/parser/gram.c. At that point I started over with a fresh REL8_2_0, ran "./configure; make", and tried the patch again. That's when I got a bunch of failures and fuzz. The problem files are: src/backend/parser/gram.c src/backend/parser/parse.h src/interfaces/libpq/libpq.rc So, I suppose libpq.rc is a derived file, also? Now I have two questions. First, why does pgcluster patch derived files? Is this just sloppy/lazy technique, or could there be some deeper reason? I realize this is properly to be posed to the pgcluster folks, but they don't seem to be too responsive, at least not to their pgfoundry forums. Second, does it make sense that the derived files that rejected the patch would be so different between the 8.2.0 tarball and my REL8_2_0 build? ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] 8.2.0 Tarball vs. REL8_2_0 vs. REL8_2_STABLE
> > difference between REL8_2_STABLE, REL8_2_0 > > STABLE doesn't mean static. It's the branch for what will be the > 8.1.x series. Okay, and this is all different from HEAD, which will presumably become 8.3, correct? ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[HACKERS] 8.2.0 Tarball vs. REL8_2_0 vs. REL8_2_STABLE (was: [GENERAL] pgcluster-1.7.0rc1-patch)
> When I apply pgcluster-1.7.0rc1-patch to Postgres REL8_2_STABLE I get > a handful of rejects. The patch applies to the 8.2.0 tarball without rejects and without fuzz. That's good. Now on to some fun with pgcluster... However, the patch will not apply to cvs branch REL8_2_0. This all raises the question: what's the difference between REL8_2_STABLE, REL8_2_0, and the 8.2.0 tarball? ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [GENERAL] Allowing SYSDATE to Work
> > Why should we add this Oraclism to PostgreSQL? I doesn't add any new > > feature. > > Certainly, this feature falls well within the class of completely > gratuitous proprietary extensions that we typically reject. I now agree completely. My purpose is to migrate Oracle databases to Posgres, and I had thought that Oracle didn't support CURRENT_DATE, CURRENT_TIMESTAMP, and so on. However, I've just learned otherwise. So, I think the proper migration process for a production database would be to first change the Oracle DB to use CURRENT_DATE (or some other standard psuedo column), since that will work properly under both Oracle and Postgres. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [GENERAL] Allowing SYSDATE to Work
> > I found it interesting that gram.c and parse.h already supported SYSDATE. > > Only after you ran bison ;-). They're derived files. Well, so much for my conspiracy theory. Thanks for the bison lesson. ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] [GENERAL] Allowing SYSDATE to Work
> I suggest you to contribute this kind of code to orafce project [1] Thanks, I'll go play over there for a while. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [GENERAL] Allowing SYSDATE to Work
> > Can't keywords share code
>
> the way to do what you want I think is
> like this:
>
> foo: bar_or_baz
> { code block }
>;
>
> bar_or_baz: bar | baz ;
I'll try that, thanks.
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [HACKERS] [GENERAL] Allowing SYSDATE to Work
Redirecting from -general.
> > I'd like SYSDATE to work syntactically and semantically the same as
> > CURRENT_TIMESTAMP
>
> current_time and the like are hardcoded in the grammar. You'd have to
> do the same for sysdate.
Okay, I patched. The patch follows. Please comment. In particular,
I've just copied the CURRENT_TIMESTAMP code block in gram.y. Is this
the best approach? I saw similar code copying between a couple of the
other time-related functions in gram.y. Can't keywords share code
blocks in bison?
I found it interesting that gram.c and parse.h already supported SYSDATE.
I patched only gram.y and keywords.c
> I'd question the hassle of having to patch all the Postgres
> installations you're going to want to run your code on.
Yeah, and I don't expect that they'll be a rush to commit this to head
anytime soon. I'll be happy enough tracking this locally. I think it's
a win for my situation.
===
RCS file: /projects/cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.568
diff -c -r2.568 gram.y
*** gram.y 5 Nov 2006 22:42:09 - 2.568
--- gram.y 17 Nov 2006 23:36:35 -
***
*** 419,425
SERIALIZABLE SESSION SESSION_USER SET SETOF SHARE
SHOW SIMILAR SIMPLE SMALLINT SOME STABLE START STATEMENT
STATISTICS STDIN STDOUT STORAGE STRICT_P SUBSTRING SUPERUSER_P SYMMETRIC
! SYSID SYSTEM_P
TABLE TABLESPACE TEMP TEMPLATE TEMPORARY THEN TIME TIMESTAMP
TO TRAILING TRANSACTION TREAT TRIGGER TRIM TRUE_P
--- 419,425
SERIALIZABLE SESSION SESSION_USER SET SETOF SHARE
SHOW SIMILAR SIMPLE SMALLINT SOME STABLE START STATEMENT
STATISTICS STDIN STDOUT STORAGE STRICT_P SUBSTRING SUPERUSER_P SYMMETRIC
! SYSDATE SYSID SYSTEM_P
TABLE TABLESPACE TEMP TEMPLATE TEMPORARY THEN TIME TIMESTAMP
TO TRAILING TRANSACTION TREAT TRIGGER TRIM TRUE_P
***
*** 7540,7545
--- 7540,7559
n->location = @1;
$$ = (Node *)n;
}
+ | SYSDATE
+ {
+ /*
+* Translate as "now()", since we have
a function that
+* does exactly what is needed.
+*/
+ FuncCall *n = makeNode(FuncCall);
+ n->funcname = SystemFuncName("now");
+ n->args = NIL;
+ n->agg_star = FALSE;
+ n->agg_distinct = FALSE;
+ n->location = @1;
+ $$ = (Node *)n;
+ }
| CURRENT_TIMESTAMP '(' Iconst ')'
{
/*
***
*** 8893,8898
--- 8907,8913
| SESSION_USER
| SOME
| SYMMETRIC
+ | SYSDATE
| TABLE
| THEN
| TO
Index: keywords.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/parser/keywords.c,v
retrieving revision 1.177
diff -c -r1.177 keywords.c
*** keywords.c 7 Oct 2006 21:51:02 - 1.177
--- keywords.c 17 Nov 2006 23:36:35 -
***
*** 324,329
--- 324,330
{"substring", SUBSTRING},
{"superuser", SUPERUSER_P},
{"symmetric", SYMMETRIC},
+ {"sysdate", SYSDATE},
{"sysid", SYSID},
{"system", SYSTEM_P},
{"table", TABLE},
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [HACKERS] cvs 'initdb' -- "vacuuming database template1 ... FATAL:
> > head does this to me when I try to initdb: > > I bet you didn't do a full recompile after "cvs update". > If you're not using --enable-depend then you really have > to do "make clean" or even "make distclean". I am using --enable-depend, but I'll 'make clean' and give it another shot. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] cvs 'initdb' -- "vacuuming database template1 ... FATAL:
> > > head does this to me when I try to initdb: > > ... > > do "make clean" or even "make distclean". > > I am using --enable-depend, but I'll "make clean" and give > it another shot. All better. Thanks. I guess I be suspicious of --enable-depend for a while. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] cvs 'initdb' -- "vacuuming database template1 ... FATAL:
> > head does this to me when I try to initdb: > > I bet you didn't do a full recompile after "cvs update". > If you're not using --enable-depend then you really have > to do "make clean" or even "make distclean". I am using --enable-depend, but I'll "make clean" and give it another shot. ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
[HACKERS] cvs 'initdb' -- "vacuuming database template1 ... FATAL: could not identify a comparison function for type aclitem"
head does this to me when I try to initdb: [EMAIL PROTECTED]:~$ /usr/local/pgsql/bin/initdb -D /usr/local/pgsql/data The files belonging to this database system will be owned by user "postgres". This user must also own the server process. The database cluster will be initialized with locale en_US.UTF-8. The default database encoding has accordingly been set to UTF8. fixing permissions on existing directory /usr/local/pgsql/data ... ok creating subdirectories ... ok selecting default max_connections ... 100 selecting default shared_buffers/max_fsm_pages ... 32MB/204800 creating configuration files ... ok creating template1 database in /usr/local/pgsql/data/base/1 ... ok initializing pg_authid ... ok initializing dependencies ... ok creating system views ... ok loading system objects' descriptions ... ok creating conversions ... ok setting privileges on built-in objects ... ok creating information schema ... ok vacuuming database template1 ... FATAL: could not identify a comparison function for type aclitem child process exited with exit code 1 initdb: removing contents of data directory "/usr/local/pgsql/data" ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] Multi-table-unique-constraint
On Friday 11 November 2005 11:07, you wrote: > It's an idea, but you are now staring directly into the hornet's nest: > > 1. How do you avoid deadlock among multiple processes all doing the >above for similar (same page anyway) keys? It's difficult if not >impossible to ensure that they'll try to take the page locks in >the same order. > Isn't all that is required is that they iterate through the indexes in the same order. This shouldn't be hard to do, because the set of indexes is the same no matter what table you are inserting into, because the unique constraint will apply to all tables both up and down the inheritance tree. That order just needs to be stored somewhere. What if there was a new system relation(pg_indexset) that stores an array of index oids. Each index that is part of an index set has an fkey into this table. When aquiring the locks on the index pages, you must a) have a ROW SHARE lock on the pg_indexset row for this set, this ensures that the schema won't change from under us. b) do so in the order that the index oids are in. This solves the problem below also, because you would hold a row exclusive lock on the row in this table whenever adding or removing indexes from the set. Now that i think about it some more, i realize that you only need to hold read locks on the index pages that you don't plan on actually inserting a new key into, which shouldn't cause near as much lock contention as holding write locks on multiple indexes' pages. > 2. What happens when another process is adding/dropping indexes that >should be in the index set? In the normal scenario you don't have >any sort of lock on any of the other tables, only the one you are >trying to insert into; and so you have no defense against somebody >changing their schemas, up to and including dropping the index you >are fooling with. Adding such locks would increase the deadlock >hazard. > Also, for many scenarios (including FKs) it's important to be able to > *look up* a particular key, not only to prevent insertion of duplicates. > The above approach would require searching multiple indexes. > Why would this be required, if it currently isn't? I mean you can already do Select from parent where key=X; and the planner takes care of scanning multiple indexes(or sequence scans). If it is required though, it should be no more difficult that doing what i described above, right? > Most of the people who have thought about this have figured that the > right solution involves a single index spanning multiple tables (hence, > adding a table ID to the index entry headers in such indexes). This > fixes the lookup and entry problems, but it's not any help for the > lock-against-schema-mods problem, and it leaves you with a real headache > if you want to drop just one of the tables. > It seems that the above solution would be less work, and would keep the data separate, which seems to be one of the biggest advantages of the current inheritance design. > 'Tis a hard problem :-( I think that's why i'm interested:) I hope that I can succeed so as to not have wasted your valuable time. BTW, i'm on the list now, so no need to cc me. Matt ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[HACKERS] Multi-table-unique-constraint
On Thursday 10 November 2005 15:58, you wrote: > >> The multi-table-unique-constraint problem has to > >> be solved before we can even think much about multi-table FKs :-( > > > > Do you have ideas about how this should be solved? > > There's some discussions in the pghackers archives --- look for > "multi-table index" and similar phrases. But if anyone had had > a really decent plan, it would have got done by now :-( > Are multi-table indexes really required? After reading the code some more, I came across this comment in nbtinsert.c:_bt_doinsert * NOTE: obviously, _bt_check_unique can only detect keys that are already in * the index; so it cannot defend against concurrent insertions of the * same key. We protect against that by means of holding a write lock on * the target page. Any other would-be inserter of the same key must * acquire a write lock on the same target page, so only one would-be * inserter can be making the check at one time. Furthermore, once we are * past the check we hold write locks continuously until we have performed * our insertion, so no later inserter can fail to see our insertion. * (This requires some care in _bt_insertonpg.) Would it be possible to make another routine that locates and aquires a write lock on the page where the key would be inserted in each index(for each table in the inheritance), and holds all these locks until the key is inserted into the correct index. It seems this would solve the unique problem without changing much else. Matt ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] pg_config --pgxs on Win32
> On 10/14/05, Dave Page wrote: > > Won't help: > > > > Microsoft Windows XP [Version 5.1.2600] > > (C) Copyright 1985-2001 Microsoft Corp. > > > > C:\Documents and Settings\dpage>mkdir c:\foo > > > > C:\Documents and Settings\dpage>cd c:/foo > > The system cannot find the path specified. > > > > C:\Documents and Settings\dpage>cd /foo > > The system cannot find the path specified. > > > > C:\Documents and Settings\dpage>cd c:\foo > > > > C:\foo> > > > > Regards, Dave > > > > Can't prepend a drive name anyways with 'cd', what if you're on a > different drive? > > =Win2kpro= > E:\>cd "c:/Test" > The system cannot find the path specified. Try "cd /D c:/Test" The /D option changes the current directory *and* the current drive in one command. -- Matt Emmerton ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] Simple tester for MVCC in PostgreSQL
On Tue, 2005-08-30 at 00:56 +0200, Martijn van Oosterhout wrote: > I saw the discussion about an tester for MVCC. Since I'd never done > anything with asyncronous queries before, I figured I'd try to write > something useful with it. The result is at: > > http://svana.org/kleptog/pgsql/mvcctest.tar.gz I've started using it in some simple cases and it seems to be a good tool. The feature set looks to me to be a pretty solid core on which to build. > It uses Perl and the Pg module from CPAN This dependency seems easy enough to live with. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Ora2Pg (was PL/pgSQL: EXCEPTION NOSAVEPOINT)
On Fri, 2005-09-02 at 12:29 -0700, Josh Berkus wrote:
> > still trying to hold on to my fantasy that I can hack Postgres (and
> > contrib/ora2pg) into submission.
>
> I'm happy to work with you on ora2pg
Cool.
It looks like I should have referred to contrib/oracle, not
contrib/ora2pg, but you got my point.
The latest version I found of ora2pg is at
http://www.samse.fr/GPL/ora2pg/ora2pg-3.3.tar.gz This seems to be more
recent than the version at contrib/oracle. For example, this newer
version has tablespace support. Given this as a starting point, I've
made the attached changes. Mostly I've added a few new config options,
but I also made a correction to the existing EXCLUDE option, and I
corrected a couple spelling/English errors along the way.
A big thing that's lacking is conversion for stored procedures and
functions. My initial approach to this was to use Perl to post-process
the PL/SQL code dumped by the export, making it look more like proper
Pl/pgSQL (e.g. VARCHAR2->VARCHAR). I'm no Perl hacker, and when I came
across significant PL/SQL <--> PL/pgSQL differences (e.g. PL/pgSQL
exception == rollback), I added to my approach the idea of hacking
PL/pgSQL to make it look more like PL/SQL. Attacking the problem from
both ends like this, I imagined that Nirvana would be reached somewhere
in the middle.
The beginning of my Perl-based attempt to convert PL/SQL into PL/pgSQL
is a pretty simple stand-alone script. I can send it if you like, but
I'm a Perl newbie, so you can probably do much better. My attempts to
make PL/pgSQL look like PL/SQL have been posted to -hackers and -patches
over the last couple months.
diff -c ora2pg_3.3/ora2pg.conf ora2pg/ora2pg.conf
*** ora2pg_3.3/ora2pg.conf 2004-12-24 16:05:40.0 +
--- ora2pg/ora2pg.conf 2005-09-02 20:38:48.900376220 +
***
*** 56,61
--- 56,68
# Value must be a list of table name separated by space.
#EXCLUDE OTHER_TABLES
+ # Set whether to include invalid functions, procedures, and packages.
+ # Under Oracle's on-the-fly invalidation/recompilation model there
+ # may be any number of objects that have status of 'INVALID' but that
+ # are actually viable.
+ INCLUDE_INVALID 1
+
+
# Display table indice and exit program (do not perform any export)
SHOWTABLEID 0
***
*** 139,148
# Constraints will be checked at the end of each transaction.
DEFER_FKEY 0
! # If set to 1 replace portable numeric type into PostgreSQL internal type.
# Oracle data type NUMBER(p,s) is approximatively converted to smallint,
# integer, bigint, real and float PostgreSQL data type. If you have monetary
# fields you should preserve the numeric(p,s) PostgreSQL data type if you need
! # very good precision. NUMBER without precision are set to float.
! PG_NUMERIC_TYPE 1
--- 146,171
# Constraints will be checked at the end of each transaction.
DEFER_FKEY 0
! # If set to 1 replace portable numeric type with PostgreSQL internal type.
# Oracle data type NUMBER(p,s) is approximatively converted to smallint,
# integer, bigint, real and float PostgreSQL data type. If you have monetary
# fields you should preserve the numeric(p,s) PostgreSQL data type if you need
! # very good precision (see PG_INTEGER_TYPE). NUMBER without precision are set to
! # float.
! PG_NUMERIC_TYPE 0
!
! # If set to 1 replace portable numeric type with PostgreSQL internal type,
! # for integers only. This behaves as PG_NUMERIC_TYPE with respect to
! # Oracle data type NUMBER(p), but preserves exact arithmetic on NUMBER(p,s)
! # columns by converting to PostgreSQL numeric(p,s). NUMBER without precision
! # maps to "numeric" without precision.
! PG_INTEGER_TYPE 1
!
! # If set to 1 map Oracle's DATE type to PostgreSQL DATE type. Oracle DATE type
! # can contain time information, so PostgreSQL "timestamp" should, in general, be
! # used to hold Oracle DATEs. However, Oracle also supports TIMESTAMP. Setting
! # PG_DATE_TYPE indicates that Oracle TIMESTAMPs are the only incoming date columns
! # with a time portion that needs to be preserved, and that incoming Oracle DATEs
! # effectively contain only a date portion.
! PG_DATE_TYPE 1
diff -c ora2pg_3.3/ora2pg.pl ora2pg/ora2pg.pl
*** ora2pg_3.3/ora2pg.pl 2004-12-24 16:05:40.0 +
--- ora2pg/ora2pg.pl 2005-07-07 18:01:53.0 +
***
*** 40,45
--- 40,46
#tables => [EMAIL PROTECTED]'TABLES'}},
tables => $Config{'TABLES'},
exclude => $Config{'EXCLUDE'},
+ include_invalid => $Config{'INCLUDE_INVALID'} || 0,
showtableid => $Config{'SHOWTABLEID'} || 0,
min => $Config{'MIN'} || 0,
max => $Config{'MAX'} || 0,
***
*** 56,66
fkey_deferrable => $Config{'FKEY_DEFERRABLE'} || 0,
defer_fkey => $Config{'DEFER_FKEY'} || 0,
pg_numeric_type => $Config{'PG_NUMERIC_TYPE'} || 0,
);
exit 0 if ($Config{'SHOWTABLEID'});
! # Mofify export structure if required
if ($Config{'TYPE'} =~ /^(DATA|C
Re: [HACKERS] PL/pgSQL: EXCEPTION NOSAVEPOINT
> > Rewriting all my Oracle code function-by-function could be painful > > ... > > I'm still trying to hold on to my fantasy that I can hack Postgres (and > > contrib/ora2pg) into submission. > > Why don't you just use EnterpriseDB? I looked at EnterpriseDB a few months ago. The installation errored. It left stuff in /var/opt, which I consider non-standard for a Red Hat machine. The whole product just didn't feel clean to me. I admit that's a pretty limited and subjective evaluation, especially for a beta product, but I was in the mode of broadly evaluating alternatives, so I moved on. Maybe I need to look at it again. Basically I feel more secure tracking the core project, even if I need to maintain some of my own patches. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] PL/pgSQL: EXCEPTION NOSAVEPOINT
On Thu, 2005-09-01 at 18:28 -0400, Tom Lane wrote: > Matt Miller <[EMAIL PROTECTED]> writes: > > Basically I'd like my Pl/pgSQL code to be able to utilize the try/catch > > paradigm of error handling without the overhead of subtransactions > > [Pl/pgSQL] can't even do 2+2 without > calling the main executor --- and recovering from elog(ERROR) without a > transaction rollback is not part of the executor's contract. Okay, so that's the crux regarding PL/pgSQL. > You might take a look at the other PLs such as plperl That would defeat my goal of not rewriting all my Oracle code. If I were fool enough to plan an attack on the main executor's exception handling to try and disarm it of its subtransaction semantics, where would I start? Where would I end? What would I do in between? Can New Orleans be rebuilt above sea level? Seriously, though, I'm willing to devote considerable time to this. Rewriting all my Oracle code function-by-function could be painful, and I would end up dragging other people around this company into it. I'm still trying to hold on to my fantasy that I can hack Postgres (and contrib/ora2pg) into submission. In the end I'm hoping that the move from Oracle will be made easier for others. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
[HACKERS] PG_PAGE_LAYOUT_VERSION - Should be Documented as 3?
doc/src/sgml/storage.sgml says: "The last 2 bytes of the page header, pd_pagesize_version, store both the page size and a version indicator. Beginning with PostgreSQL 8.0 the version number is 2; PostgreSQL 7.3 and 7.4 used version number 1; prior releases used version number 0." But src/include/storage/bufpage.h says: "/* * Page layout version number 0 is for pre-7.3 Postgres releases. * Releases 7.3 and 7.4 use 1, denoting a new HeapTupleHeader layout. * Release 8.0 changed the HeapTupleHeader layout again. * Release 8.1 redefined HeapTupleHeader infomask bits. */ #define PG_PAGE_LAYOUT_VERSION 3" So, should the attached be applied? Index: storage.sgml === RCS file: /var/local/pgcvs/pgsql/doc/src/sgml/storage.sgml,v retrieving revision 1.6 diff -c -r1.6 storage.sgml *** storage.sgml 28 Apr 2005 21:47:09 - 1.6 --- storage.sgml 1 Sep 2005 15:32:35 - *** *** 437,443 The last 2 bytes of the page header, pd_pagesize_version, store both the page size and a version indicator. Beginning with ! PostgreSQL 8.0 the version number is 2; PostgreSQL 7.3 and 7.4 used version number 1; prior releases used version number 0. (The basic page layout and header format has not changed in these versions, --- 437,444 The last 2 bytes of the page header, pd_pagesize_version, store both the page size and a version indicator. Beginning with ! PostgreSQL 8.1 the version number is 3; ! PostgreSQL 8.0 used version number 2; PostgreSQL 7.3 and 7.4 used version number 1; prior releases used version number 0. (The basic page layout and header format has not changed in these versions, ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] 8.1 and syntax checking at create time
On Wed, 2005-08-31 at 15:29 -0400, Tom Lane wrote: > Matt Miller <[EMAIL PROTECTED]> writes: > > I don't remember the last time I intended to write code that referenced > > something that did not exist in the database. > > Almost every day, people try to write stuff like > > CREATE TEMP TABLE foo ... ; > INSERT INTO foo ... ; > etc etc > DROP TABLE foo ; Point taken. PL/SQL requires all DDL to be dynamic SQL. For example: execute immediate 'drop table foo'; The stuff inside the string is pretty-much ignored at compile time. Maybe, then, my idealized PL/pgSQL compiler always allows DDL to reference any object, but DML is checked against the catalog. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] 8.1 and syntax checking at create time
On Wed, 2005-08-31 at 11:59 -0700, Josh Berkus wrote: > If a table does not exist, we don't want to check for that and bounce > the function; possibly the function will only be called in a context > where the table does exist. The Pl/pgSQL compiler should be able to dive into SQL statements, hit the catalog, and bounce a function because of invalid database object references. Ideally this capability could be turned off on demand. I am thankful that Oracle's PL/SQL compiler checks these things for me. I don't remember the last time I intended to write code that referenced something that did not exist in the database. I agree,though, that some developers might rely on such a capability in some circumstances. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] 8.1 and syntax checking at create time
On Wed, 2005-08-31 at 13:13 -0500, Tony Caduto wrote: > the function below also raises no errors at create, but at run time it does. > ... > CREATE or REPLACE FUNCTION public.test_func9(out firstname varchar,out > lastname varchar) > RETURNS SETOF pg_catalog.record AS > $BODY$ > Declare > row record44; > BEGIN > asfdfdfdfafdsfsdfsdf > sdf bla bla > sdf yada yada > s > df > sd > fsd > END; > $BODY$ > LANGUAGE 'plpgsql' VOLATILE; When I execute this CREATE statement I get: ERROR: type "record44" does not exist CONTEXT: compile of PL/pgSQL function "test_func9" near line 2 So, it does seem to be working as advertised. I'm running HEAD as of a few hours ago. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Call for 7.5 feature completion
On Fri, 2005-08-26 at 13:13 -0400, Nicholas Walker wrote: > >You can't use savepoints, you can trap errors which is implemented using > >savepoints. You still might want to write code like this: > > > >BEGIN > > > > > > > >SAVEPOINT foo; > > > > > > > >IF SOME_ERROR_CODE = 1234 THEN > > ROLLBACK TO SAVEPOINT foo; > >END > > > >... > I agree, and I think savepoints would be much more usefull if you could > call them from pl/pgsql... Maybe if PL/pgSQL had user-defined exceptions then the language's identity of savepoints and exception blocks would be a little easier to work with. Is anything happening with the patch for user-defined exceptions, posted at http://archives.postgresql.org/pgsql-patches/2005-06/msg00475.php (and also discussed elsewhere)? ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] TODO list comments
On Thu, 2005-08-25 at 15:50 +0900, Michael Glaesemann wrote:
> >> * %Remove CREATE CONSTRAINT TRIGGER
> >>
> > Do we really want to remove it,
>
> Also, I believe CONSTRAINT TRIGGERS are the only way to provide
> transaction level (rather than statement level) referential
> integrity.
Don't deferrable foreign keys give you transaction-level referential
integrity? From the SET CONSTRAINTS doc:
Synopsis
SET CONSTRAINTS { ALL | name [, ...] } { DEFERRED | IMMEDIATE }
Description
SET CONSTRAINTS sets the behavior of constraint checking within the
current transaction. IMMEDIATE constraints are checked at the end of
each statement. DEFERRED constraints are not checked until transaction
commit.
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [HACKERS] Testing of MVCC
> > Perhaps we should look at Expect or something similar. > > Where can I get more info on Expect? I think I found it: http://expect.nist.gov/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Testing of MVCC
> What we really need is a test program that can issue a command on one > connection (perhaps waiting for it to finish, perhaps not) and then > issue other commands on other connections, all according to a script. It seems to me that this is what contrib/dblink could allow, but when I presented that idea earlier you replied: > I doubt it would be very useful, since a script based on that > still doesn't let you issue concurrent queries. So, I guess I'm not clear on what you're thinking. > Perhaps we should look at Expect or something similar. Where can I get more info on Expect? ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Testing of MVCC
On Mon, 2005-08-08 at 16:59 -0400, Tom Lane wrote:
> Matt Miller <[EMAIL PROTECTED]> writes:
> > I want to write some regression tests that confirm the behavior of
> > multiple connections simultaneously going at the same tables/rows. Is
> > there something like this already, e.g. in src/test/regress?
>
> No. ... but surely we need one.
The attached patch allows src/test/regress/pg_regress.sh to recognize
lines that begin with "curr_test:" in the schedule file. Tests named on
such a line are run concurrently across multiple connections. To make
use of this facility each test in the group must begin with the line:
select * from concurrency_test where key = '' for update;
where is replace by the name of that test. This will enable
pg_regress to start this test at the same time as the other tests in the
group.
Is this a reasonable starting point for a concurrent testing framework?
This does not address the issue of how to interpret the test output.
Maybe the simplest solution is to force test writers to generate output
that does not depend on the relative progress of any concurrent tests.
Or, maybe the "ignore:" directive in the schedule file could be employed
somehow.
Index: pg_regress.sh
===
RCS file: /var/local/pgcvs/pgsql/src/test/regress/pg_regress.sh,v
retrieving revision 1.59
diff -c -r1.59 pg_regress.sh
*** pg_regress.sh 17 Jul 2005 18:28:45 - 1.59
--- pg_regress.sh 15 Aug 2005 21:20:03 -
***
*** 623,628
--- 623,632
do
# Count line numbers
lno=`expr $lno + 1`
+
+ # Init concurrency flag
+ concurrent=
+
[ -z "$line" ] && continue
set X $line; shift
***
*** 631,636
--- 635,647
shift
ignore_list="$ignore_list $@"
continue
+ elif [ x"$1" = x"curr_test:" ]; then
+ # init support for concurrent test group
+ concurrent=1
+ cat /dev/null >"$inputdir/sql/concurrency_test_init.sql"
+ echo "create table concurrency_test (key varchar primary key);" >>"$inputdir/sql/concurrency_test_init.sql"
+ ( $PSQL -d "$dbname" <"$inputdir/sql/concurrency_test_init.sql" >"$outputdir/results/concurrency_test_init.out" 2>&1 )&
+ wait
elif [ x"$1" != x"test:" ]; then
echo "$me:$schedule:$lno: syntax error"
(exit 2); exit
***
*** 649,671
( $PSQL -d "$dbname" <"$inputdir/sql/$1.sql" >"$outputdir/results/$1.out" 2>&1 )&
wait
else
! # Start a parallel group
! $ECHO_N "parallel group ($# tests): $ECHO_C"
! if [ $maxconnections -gt 0 ] ; then
! connnum=0
! test $# -gt $maxconnections && $ECHO_N "(in groups of $maxconnections) $ECHO_C"
! fi
! for name do
! (
! $PSQL -d "$dbname" <"$inputdir/sql/$name.sql" >"$outputdir/results/$name.out" 2>&1
! $ECHO_N " $name$ECHO_C"
! ) &
if [ $maxconnections -gt 0 ] ; then
! connnum=`expr \( $connnum + 1 \) % $maxconnections`
! test $connnum -eq 0 && wait
fi
! done
! wait
echo
fi
--- 660,717
( $PSQL -d "$dbname" <"$inputdir/sql/$1.sql" >"$outputdir/results/$1.out" 2>&1 )&
wait
else
! # --
! # If this is a concurrent test group then write the script "concurrent_test.sql"
! # which will spawn and synchronize each test in the group.
! #
! # Concurrent test groups do not respect $maxconnections.
! #
! # If this is not a concurrent test group then just run each test directly.
! # --
!
! if [ "$concurrent" = "1" ]; then
! $ECHO_N "concurrent group ($# tests): $ECHO_C"
!
! # insert a lock record for each test
! cat /dev/null >"$inputdir/sql/concurrency_test.sql"
! echo "BEGIN;" >>"$inputdir/sql/concurrency_test.sql"
! for name do
! echo "insert into concurrency_test values ('$name');" >>"$inputdir/sql/concurrency_test.sql"
! done
! echo "COMMIT;" >>"$inputdir/sql/concurrency_test.sql"
!
! # for each test, acquire the lock and then spawn the test
! echo "BEGIN;" >>"$inputdir/sql/conc
Re: [HACKERS] [GENERAL] Testing of MVCC
On Wed, 2005-08-10 at 16:41 -0400, Tom Lane wrote: > Matt Miller <[EMAIL PROTECTED]> writes: > > It seems to me that contrib/dblink could greatly simplify the design and > > coding of multi-user regression tests. > > I doubt it would be very useful, since > a script based on that still doesn't let you issue concurrent queries. I think it would be useful to allow a test script to first create a set of committed and uncommitted transactions, and to then issue some queries on another connection to confirm that the other connection has a proper view of the database at that point. This type of test is serialized, but I think it would be a useful multi-user test. Also, the output from such a test is probably pretty easy to fit into the diff-based validation of "make check." I realize that we also need to have tests that spawn several connections and run scripts concurrently across those connections. I agree that this type of test would probably not benefit fundamentally from contrib/dblink. However, I was grasping a bit to see how the output from such a concurrent test would be diff'ed with an expected output in a meaningful way. So, to continue to progress on this problem, I figured that a contrib/dblink dependency would at least allow me to start coding something... > > Is there objection to a portion > > of src/test/regress depending on contrib/dblink? > > Yes. Understood. ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [GENERAL] Testing of MVCC
On Mon, 2005-08-08 at 16:59 -0400, Tom Lane wrote: > Matt Miller <[EMAIL PROTECTED]> writes: > > I want to write some regression tests that confirm the behavior of > > multiple connections simultaneously going at the same tables/rows. Is > > there something like this already, e.g. in src/test/regress? > > No. ... but surely we need one. It seems to me that contrib/dblink could greatly simplify the design and coding of multi-user regression tests. Is there objection to a portion of src/test/regress depending on contrib/dblink? I'm not sure yet how that dependency would look, but I'm mainly wondering if there are objections in principle to depending on contrib/. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] ORDER BY
> On Mon, Jul 25, 2005 at 06:11:08PM -0300, Marc G. Fournier wrote: > > > > Just curious as to whether or not a warning or something should be issued > > in a case like: > > > > SELECT c.* > > FROM company c, company_summary cs > >WHERE c.id = cs.id > > AND cs.detail = 'test' > > ORDER BY cs.fullname; > > > > Unless I'm missing something, the ORDER BY clause has no effect, but an > > EXPLAIN shows it does take extra time, obviously ... > > Uh, I'd hope it had an effect. Note that RDBMSes have been moving > towards allowing fields in ORDER BY that aren't in the SELECT list, > though in the past it was common that anything in ORDER BY had to also > be in SELECT. Prior to SQL:1999, the spec required that any column referenced in an ORDER BY clause must also be referenced in the SELECT. SQL:1999 (feature E1210-02) relaxed this to allow columns to be specified in the ORDER BY clause but not in the SELECT. -- Matt Emmerton ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] patch: plpgsql - access records with rec.(expr)
> I think the idea of rewriting PL/PgSQL from scratch has merit (and it's > something that I think would be well worth doing). IMHO it's not really > worth the trouble to fork the existing code base and add new features to > something that, hopefully, has a limited life span. I dunno, I kind of like the idea. There's always going to be the age old conflict between people who are basically users like me, and want to see a needed feature asap, and developers who want to see it done right. And of course doing it right is only way to go long term. But so long as I can be fairly sure the syntax is agreed (EXECUTE SELECT INTO ... in this case) and eventually will make it into the main code base, I'd be willing to live on the edge for a while. There'd have to be big 'experimental, everything may change' warnings all over the contrib version. My only concern is if it would actually delay the discussed rewrite of plpgsql by splitting the effort. That's my two one hundreths of a euro, anyway. Matt ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] patch: plpgsql - access records with rec.(expr)
> What about > for i in ... > ... new.(tg_argv[i]) ... Ooof! Constants or digits or nothing, then > MHO: this is a really ugly wart on the language, and it does not solve > the problems people would want to solve. It might solve *your* problem > but that's not enough to justify a wart of this size. But my warts are beautiful! OK, fair enough. I had to try. > We do need to do something about the fact that EXECUTE can't access > plpgsql variables, though I'm afraid that fixing that is going to > require a rather complete overhaul of plpgsql :-(. But it needs one > anyway. Look forward to seeing it. Matt ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] patch: plpgsql - access records with rec.(expr)
> > See your point. But what about NEW.($1)? > > I don't follow -- what do you mean? I want to be able to be able to write a trigger function that accesses a column passed as an argument to the function in the row that caused the trigger. This is my use case. I guess that would actually written NEW.(TG_ARGV[1]). > (BTW, I think my comment also applies to variables of type "text" and > similar -- I think the patch would be a lot simpler if you just > implement access to record fields by ordinal position, and don't > implement access by field name.) Yes, it would be marginally simpler: I'd still have to call exec_eval_datum() on the variable and check whether it could be evaluated to an integer (trigger args are all text AFAIK). The only difference would be throwing an error if it wasn't, instead of making use of the value... and a slightly less readable 'create trigger' statement. It would be a good idea to check that the variable was either a constant or a trigger arg. This would stop the looping problem, since the type of the underlying field couldn't change. But I've somehow got the feeling that this sort of thing isn't the issue. The issue is whether we want to allow dynamic access to columns in any syntax at all. A simple yes or no would do :) Matt BTW: here's the original post adding the rec.(3) syntax to the TODO: http://archives.postgresql.org/pgsql-hackers/2003-07/msg00425.php here's someone else who tried something very similar: http://archives.postgresql.org/pgsql-hackers/2003-09/msg00533.php ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] patch: plpgsql - access records with rec.(expr)
Hi Tom, > > Does that make any sense? Is it worth the work? Or should we just tell > > anyone who actually needs it (I don't, at present) 'use another PL'? > > I don't really see this going anywhere --- it's contorting the semantics > of plpgsql too much for too little gain. Yup, the last bit was definitely contortionist! Prob should have split it into two separate posts. What I'm most looking for comments on what's actually in the patch: with it I can use NEW.($1). _That_ is a big gain. On the rest... > Your proposal doesn't address the > issue of how the function would find out the column names in order to > make use of the proposed notation; It doesn't need them. The posted patch already allows: for i in 1..3 loop myvar := rec.(i); end loop; ...so long as all cols have the same datatype (note that col numbers are 1-based, like in the spi funcs). > and as you noted there's still a > serious problem with varying datatypes. My contortions were an attempt to think my way around the varying datatypes problem. Obviously my thinking got tied in knots ;) I agree, that part is too much work for too little gain. > Either plperl or pltcl (probably also plpython but I'm not familiar > with that language) is a better choice for writing such triggers, > because those languages already have answers for all these issues. I didn't think plperl could be used to write triggers. Is this new in 8? As an application developer, though, it'd be nice to keep everything in one pl language, and ideally one guaranteed available (or easily installable without other dependencies) on every postgres db out there. That's pretty much why I wrote the patch: to add the one missing feature I need to write my generic triggers in plpgsql. Regards, Matt ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [HACKERS] patch: plpgsql - access records with rec.(expr)
Hi,
Updated patch (now against beta5) attached. It now pfree's any converted
strings, avoids pointlessly casting an int4oid to int4oid, complies to
CS (uses tabs, not spaces) and works with label.record.(expression) too.
I'm still testing, it now does what I set out to achieve.
I haven't done anything to implement the same thing for row types. I'm
wondering if there's any point. The person writing the function is
implicitly going to know the structure of a row; the only use I can
foresee is iterating over the columns by ordinal, but that isn't
possible (at present, see below).
Neil's raised a coupe of good points:
- is the parenthesis format the right way to go? How do other sp
languages do it?
- if we have a more genaralised EVALUATE statement, do we still need
this?
I'm not at all fixated on the parenthesis idea. If there's a better way,
great. But if this stands a chance of making it in to the language one
day, it'd be good to know so I can start building stuff that uses the
format.
One thought I had about the format would be to use a kind of
javascript-ish syntax:
rec.item(expr) - as above
This might later lend itself to other record-specific functions:
rec.length() - returns number of columns
rec.getColType(expr) - returns oid of given column
rec.getColNumber(expr) - returns ordinal of given column
I can't see a crying need for any of these, mind... but length() would
be useful _if_ you could iterate over columns, so:
Loops:
The issue with loops is a nuisance - if the underlying column type
changes, my stuff throws a data type mismatch exception. I imagine this
could affect an EVALUATE expression inside a loop, too (or would their
plans never be saved, like EXECUTE?).
I might be totally wrong, but here's what I see:
To stop the mismatch you'd have to stop exec_eval_expr() saving the plan
for that expression. A flag in the expression struct could do that,
though you'd want to add a check that the function was defined as
volatile before allowing it.
The flag could be set during the function's compilation stage, but then
all functions using the rec.(expr) format anywhere would have to be
volatile.
A better approach would be to walk through the function's statement tree
just before execution, checking for troublesome expressions appearing
inside loops (and whose arguments are reassigned inside the loop?) and
marking them as dynamic.
Does that make any sense? Is it worth the work? Or should we just tell
anyone who actually needs it (I don't, at present) 'use another PL'?
Regards,
Matt
diff -u src.orig/pl_comp.c src/pl_comp.c
--- src.orig/pl_comp.c 2004-11-22 10:20:24.0 +
+++ src/pl_comp.c 2004-11-22 10:58:42.465929360 +
@@ -783,6 +783,72 @@
return T_WORD;
}
+/* --
+ * plpgsql_parse_wordexpr Same lookup for word followed by dot.
+ *Should only get here if it wasn't followed by
+ *an identifier.
+ * --
+ */
+int
+plpgsql_parse_wordexpr(char *word)
+{
+ PLpgSQL_nsitem *ns;
+ char *cp[1];
+ int save_spacescanned = plpgsql_SpaceScanned;
+
+ /* Do case conversion and word separation */
+ /* drop dot to keep converter happy */
+ word[strlen(word) - 1] = '\0';
+ plpgsql_convert_ident(word, cp, 1);
+
+ /*
+ * Do a lookup on the compilers namestack
+ */
+ ns = plpgsql_ns_lookup(cp[0], NULL);
+ if (ns == NULL)
+ {
+ pfree(cp[0]);
+ return T_ERROR;
+ }
+ switch (ns->itemtype)
+ {
+ case PLPGSQL_NSTYPE_REC:
+ {
+/*
+ * First word is a record name, so the parenthesised expr that
+ * follows it defines the field.
+ */
+PLpgSQL_recfield *new;
+
+new = malloc(sizeof(PLpgSQL_recfield));
+memset(new, 0, sizeof(PLpgSQL_recfield));
+
+if (plpgsql_yylex() != '(')
+ plpgsql_yyerror("expected identifier or \"(\"");
+new->expr = plpgsql_read_expression(')', ")");
+new->recparentno = ns->itemno;
+new->isExpr = true;
+new->dtype = PLPGSQL_DTYPE_RECFIELD;
+
+plpgsql_adddatum((PLpgSQL_datum *) new);
+
+plpgsql_yylval.scalar = (PLpgSQL_datum *) new;
+
+plpgsql_SpaceScanned = save_spacescanned;
+
+pfree(cp[0]);
+return T_SCALAR;
+ }
+
+ /* TODO: deal with rows, too? */
+
+ default:
+ break;
+
+ }
+ pfree(cp[0]);
+ return T_ERROR;
+}
/* --
* plpgsql_parse_dblword Same lookup for two words
@@ -855,6 +921,7 @@
new->dtype = PLPGSQL_DTYPE_RECFIELD;
new->fieldname = strdup(cp[1]);
new->recparentno = ns->itemno;
+new->isExpr = false;
plpgsql_adddatum((PLpgSQL_datum *) new);
@@ -901,6 +968,89 @@
return T_ERROR;
}
+/* --
+ * plpgsql_parse_dblwordexpr Same lookup for two words
+ * separated by a dot followed by a lone dot
+ * --
+ */
+int
+plpgsql_parse_dblwordexpr(char *word)
+{
+ PLpgSQL_nsitem
Re: [HACKERS] patch: plpgsql - access records with rec.(expr)
Hi Neil,
Thanks for the comments. I've actually got (yet) another version ready
to go, which fixes the compile warnings and adds some sanity checks.
I'll post it as soon as I've got beta5 downloaded and tried out :)
> FYI, one thing I want to implement is an EVALUATE statement in plpgsql
> (analogous to eval() in Perl, for example). If I understand your use
> case, I think this will help somewhat, although of course it is still
> clumsier than direct syntactic support.
This would execute a string and pass back the result? I'm sure I'll find
a use for it at some point :)
> > rec.('foo')
>
> I don't like this: it implicitly coerces a string literal into an
> identifier (i.e. a column name). Treating data as code can be useful,
> but I think we need to make it more obvious to the user. I think a
> proper EVALUATE statement might be a better solution.
See your point. But what about NEW.($1)?
> > 5. Because of the way the expression is parsed (looking for closing
> > parenth), this will choke if you try and put a function in there. Would
> > it be better to use curly braces '{expr}' or another character to mark
> > the expression?
>
> How much thought went into choosing parentheses? (i.e. is a similar
> syntax used in the procedural languages in other DBs?)
Only used them because of the small note I saw on the developer's TODO
about accessing cols by ordinal - the example there was rec.(1). But I
was wrong about functions not working there - plpgsql_read_expression()
is smarter than that, as you say.
OK, download is done. I've got some more general ideas which relate to
this. I'll post along with updated version.
Matt
---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faqs/FAQ.html
Re: [HACKERS] patch: plpgsql - access records with rec.(expr)
Hi all,
I've cleaned this up a bit so that assigning to a dynamic record field
now works - ie NEW.(myvar) := 'someval' - and accessing a field by
number works - ie myvar := OLD.(1)
It still doesn't work in a loop if the type of field referenced by the
expression changes - looking at it more I'm really not sure this is
possible, but that's a limitation I could live with. I'll also try
figuring out freeing stuff after casting values over the weekend.
But is this a worthwhile approach? If there are objections, please let
me know!
For myself, being able to pass a column name as an argument to a trigger
would make writing generic triggers a whole lot easier. And going back
through the archives on this list I see I'm not alone.
Regards,
Matt
On Thu, 2004-11-18 at 13:18, Matt wrote:
> Hi,
>
> I got extremely frustrated with having to create a temp table every time
> I wanted to access an arbitrary column from a record plpgsql. After
> seeing a note on the developers TODO about accessing plpgsql records
> with a 'dot bracket' notation I started digging into the plpgsql source.
>
> My diff (against 8beta4) is attached.
>
> Warning: I Am Not a C Programmer! I haven't even written a hello world
> in C before, and I knew nothing about Flex before yesterday. It was fun
> figuring stuff out, I'm amazed it mostly works, but I'm really hoping
> someone can point out my mistakes.
>
> Goal:
>
> Enable users to access fields in record variables using the following
> syntax like the following:
> rec.(1)
> rec.('foo')
> rec.(myvar::int)
> rec.(myvar || '_id')
>
> Files changed:
>
> plpgsql.h
> - added 'expr' member to PLpgSQL_recfield type for holding the
> PLpgSQL_expr structure.
>
> scan.l
> - added match for {identifier}{space}*\. AFAIK this should only match
> if a longer expression doesn't?
>
> pl_comp.c
> - added plpgsql_parse_wordexpr() function called by above match. Ripped
> off code from plpgsql_parse_word that deals with arg_v[expr] to find our
> expression. Prob a dumb name for the function!
>
> pl_exec.c
> - hacked exec_assign_value() and exec_eval_datum() to use the expression
> to get the field name/number.
>
> Stuff I need help with:
>
> 1. It should recognise OLD.(1) as a field number, not a column name. I
> think I've got to check the returned type from exec_eval_expr() then
> exec_simple_cast_value() on it, but that seems beyond me.
>
> 2. Freeing stuff. As I explained, this is all pretty new to me, and the
> comments about it around exec_eval_expr() et al just confused me :(
> Please give some hints about what needs freeing!
>
> 3. Would probably be good to add check in pl_comp.c to see if the
> expression actually needs to be evaluated at runtime (ie isn't just a
> field name/number). How?
>
> 4. Make this also work for row.(expr), label.record.(expr) and
> label.row.(expr) - but want to get the basics working first!
>
> 5. Because of the way the expression is parsed (looking for closing
> parenth), this will choke if you try and put a function in there. Would
> it be better to use curly braces '{expr}' or another character to mark
> the expression?
>
> I hope at this eventually leads to some really useful extra
> functionality, particularly for writing generic triggers. And it's a
> tribute to the existing code that a complete newbie can cut-and-paste
> their way to a halfarsed solution in a (rather long) night!
>
> Regards,
>
> Matt
>
> __
>
> ---(end of broadcast)---
> TIP 9: the planner will ignore your desire to choose an index scan if your
> joining column's datatypes do not match
diff -u src.orig/pl_comp.c src/pl_comp.c
--- src.orig/pl_comp.c 2004-11-19 14:01:15.053237796 +
+++ src/pl_comp.c 2004-11-19 14:23:04.192899809 +
@@ -783,6 +783,75 @@
return T_WORD;
}
+/* --
+ * plpgsql_parse_wordexpr Same lookup for word followed by dot.
+ * Should only get here if it wasn't followed by
+ * an identifier.
+ * --
+ */
+int
+plpgsql_parse_wordexpr(char *word)
+{
+ PLpgSQL_nsitem *ns;
+ char *cp[1];
+int save_spacescanned = plpgsql_SpaceScanned;
+
+ /* Do case conversion and word separation */
+ /* drop dot to keep converter happy */
+word[strlen(word) - 1] = '\0';
+plpgsql_convert_ident(word, cp, 1);
+
+/* Make sure we've got an open parenthesis */
+
+/*
+ * Do a lookup on the compilers namestack
+ */
+ ns = plpgsql_ns_looku
Re: [HACKERS] patch: plpgsql - access records with rec.(expr)
> 5. Because of the way the expression is parsed (looking for closing
> parenth), this will choke if you try and put a function in there. Would
> it be better to use curly braces '{expr}' or another character to mark
> the expression?
I lie! pgpgsql_read_expression() is smarter than that!
However, I do have another problem. If the value of the expr changes
inside a loop to a fieldname of a different type, it dies with the "type
of \"%s\" does not match that when preparing the plan" message, which is
quite true: it obviously doesn't.
Just setting expectedtypeoid to InvalidOid bombs the whole thing :(
Hrm.... the "best made plans" and all that...
Matt
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match
[HACKERS] patch: plpgsql - access records with rec.(expr)
Hi,
I got extremely frustrated with having to create a temp table every time
I wanted to access an arbitrary column from a record plpgsql. After
seeing a note on the developers TODO about accessing plpgsql records
with a 'dot bracket' notation I started digging into the plpgsql source.
My diff (against 8beta4) is attached.
Warning: I Am Not a C Programmer! I haven't even written a hello world
in C before, and I knew nothing about Flex before yesterday. It was fun
figuring stuff out, I'm amazed it mostly works, but I'm really hoping
someone can point out my mistakes.
Goal:
Enable users to access fields in record variables using the following
syntax like the following:
rec.(1)
rec.('foo')
rec.(myvar::int)
rec.(myvar || '_id')
Files changed:
plpgsql.h
- added 'expr' member to PLpgSQL_recfield type for holding the
PLpgSQL_expr structure.
scan.l
- added match for {identifier}{space}*\. AFAIK this should only match
if a longer expression doesn't?
pl_comp.c
- added plpgsql_parse_wordexpr() function called by above match. Ripped
off code from plpgsql_parse_word that deals with arg_v[expr] to find our
expression. Prob a dumb name for the function!
pl_exec.c
- hacked exec_assign_value() and exec_eval_datum() to use the expression
to get the field name/number.
Stuff I need help with:
1. It should recognise OLD.(1) as a field number, not a column name. I
think I've got to check the returned type from exec_eval_expr() then
exec_simple_cast_value() on it, but that seems beyond me.
2. Freeing stuff. As I explained, this is all pretty new to me, and the
comments about it around exec_eval_expr() et al just confused me :(
Please give some hints about what needs freeing!
3. Would probably be good to add check in pl_comp.c to see if the
expression actually needs to be evaluated at runtime (ie isn't just a
field name/number). How?
4. Make this also work for row.(expr), label.record.(expr) and
label.row.(expr) - but want to get the basics working first!
5. Because of the way the expression is parsed (looking for closing
parenth), this will choke if you try and put a function in there. Would
it be better to use curly braces '{expr}' or another character to mark
the expression?
I hope at this eventually leads to some really useful extra
functionality, particularly for writing generic triggers. And it's a
tribute to the existing code that a complete newbie can cut-and-paste
their way to a halfarsed solution in a (rather long) night!
Regards,
Matt
diff -u src/pl_comp.c src.mk/pl_comp.c
--- src/pl_comp.c 2004-09-13 21:09:20.0 +0100
+++ src.mk/pl_comp.c 2004-11-18 12:59:25.825372489 +
@@ -3,7 +3,7 @@
* procedural language
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.82 2004/09/13 20:09:20 tgl Exp $
+ * $PostgreSQL: pgsql-server/src/pl/plpgsql/src/pl_comp.c,v 1.82 2004/09/13 20:09:20 tgl Exp $
*
* This software is copyrighted by Jan Wieck - Hamburg.
*
@@ -783,6 +783,75 @@
return T_WORD;
}
+/* --
+ * plpgsql_parse_wordexpr Same lookup for word followed by dot.
+ * Should only get here if it wasn't followed by
+ * an identifier.
+ * --
+ */
+int
+plpgsql_parse_wordexpr(char *word)
+{
+ PLpgSQL_nsitem *ns;
+ char *cp[1];
+int save_spacescanned = plpgsql_SpaceScanned;
+
+ /* Do case conversion and word separation */
+ /* add fake bit after dot to keep converter happy */
+word[strlen(word) - 1] = '\0';
+plpgsql_convert_ident(word, cp, 1);
+
+/* Make sure we've got an open parenthesis */
+
+/*
+ * Do a lookup on the compilers namestack
+ */
+ ns = plpgsql_ns_lookup(cp[0], NULL);
+ if (ns == NULL)
+ {
+ pfree(cp[0]);
+ return T_ERROR;
+ }
+switch (ns->itemtype)
+{
+ case PLPGSQL_NSTYPE_REC:
+ {
+/*
+ * First word is a record name, so expression refers to
+ * field in this record.
+ */
+PLpgSQL_recfield *new;
+
+new = malloc(sizeof(PLpgSQL_recfield));
+memset(new, 0, sizeof(PLpgSQL_recfield));
+
+if (plpgsql_yylex() != '(')
+plpgsql_yyerror("expected identifier or \"(\"");
+new->expr = plpgsql_read_expression(')', ")");
+new->recparentno = ns->itemno;
+/* just to be sure - we'll test on this later */
+new->fieldname = '\0';
+new->dtype = PLPGSQL_DTYPE_RECFIELD;
+
+plpgsql_adddatum((PLpgSQL_datum *) new);
+
+plpgsql_yylval.scalar = (PLpgSQL_datum *) new;
+
+plpgsql_SpaceScanned = save_spacescanned;
+
+pfree(cp[0]);
+return T_SCALAR;
+ }
+
+/* TODO: deal with rows, too */
+
+default:
+
Re: [HACKERS] PostgreSQL as an application server
[EMAIL PROTECTED] (Ned Lilly) wrote in message news:<[EMAIL PROTECTED]>... > Jonathan, > > This is exactly how my company has built a very robust ERP application. See > www.openmfg.com. > Hi Mr. Lilly: I not sure who to address this question to at your company. I'm a valued added reseller who combines a dozen plus products, CAD engineering and project management to serve a construction niche market. Probably a third of our dollar purchases go to our primary OEM. Though they are vital to us we are probably less than 5% of their sales. Back in March 2003 I lobbied (e-mailed .pdf files and web URLs; made calls, followed up) both the company president and the general manager of that OEM to consider OpenMFG in lieu of a MRP system proposed by a Michigan based Microsoft "partner". Last week the general manager mentioned in passing that she was going back to the office this weekend to begin learning their new OpenMFG software. [ Hummm. I wish we were in "the loop" more. :>) ] Our micro business also wants to update its accounting, sales and project management software/processes. Even SOHO businesses like ourselves dispair of order entry and vendor acknowledgements via a stream of faxes. As an on-going buyer from your OpemMFG customer how can we can easily upload/download purchase orders and receive back acknowledgements? From a transaction processing standpoint faxes are utterly "brain dead". E-mailing Excel files is marginally better but still very labor intensive, error prone and not easily manipulated or indexed. This next week we will contract with a free lance programmer/analyst to update our Access '97 VBA based accounting software to an Access 2003 front end and a SQL Server 2005 Express backend (ODBC links). This involves upsizing our static customer, vendor, project and job related revenue and cost transaction data to a format we can manipulate in Access 2003 or independently. Does your "Sales Order Management" have some kind of file related batch processing that would allow us to e-mail orders to our OEM (your OpenMFG customer)? This OEM says they only need occassional dial-up for internet access so web portal data entry is not an option. That's OK. I prefer not to rekey orders on our end anyway. Presently our faxed orders to this OEM are 1-5 pages of bone wearying detail printed from Excel spreadsheets. The detail comes from macros acting on "lookup" Excel worksheets, reformated and with quantities added, extended and totaled. The "lookup" Excel worksheets are created from this OEM's product-price schedules faxed to us, OCR'd, edited and copied to Excel. Is there any way our OEM can grab a snap shot of the pertinent tables from your OpenMFG product PostgreSQL database, export them (file format?) and e-mail to us? Then we could revised our Excel quotation spreadsheets to "lookup" product/price/description/weight/dimensions/cubes/notations from SQL tables (via ODBC, OLE.DB or 3rd party software). If we get the job and the quote becomes an order we could then sort product line-items by OEM, copy/paste to Access 2003 and (hopefully) export to whatever file format will batch process back into OpenMFG's "Sales Order Management" module. For what it may be worth we've already started on the XSL/XSLT learning curve with Altova's XMLSPY 2004 Professional Edition. Though XML is intended for machine to machine interaction we use XML to keep track of sheets of different sizes and hundreds of "off cut" parts. XML is the native import/export format for the panel nesting program we use. We've gotten familar with cutting and pasting the XML data file. Does this have application? See especially http://www.altova.com/whatsnew.html#db . I'm probably being simplistic above. Hey, a micro/SOHO distributor can dream, can't they? How far through does OpenMFG (or related software) followed the "food chain" downstream? Thank you in advance for your thoughts (or referral within your company to whoever can assist us). Alanzo Manton ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
