Tom Lane Wrote:
Well, this certainly demonstrates that the check I added to
parseCheckAggregates is wrongly placed, but I'm not sure we really
need to forbid the case. David's example query seems to give sane
answers once the bug in begin_partition is fixed:
parentpart | childpart |
David Rowley dgrow...@gmail.com writes:
Also while testing I noticed that this query didn't error out when it should
have: (Of course I only noticed because Sybase did)
WITH RECURSIVE bom(parentpart,childpart,quantity,rn) AS (
SELECT parentpart,childpart,quantity,ROW_NUMBER() OVER (ORDER BY
Tom Lane Wrote:
Actually, it's not ambiguous according to our interpretation of ORDER BY
clauses: the first attempt is to match an output column name, so it's
seizing on the first SELECT column (b.parentpart) as being the intended
sort key for parentpart, and similarly for childpart. You'd
Tom Lane Wrote:
I've spent quite a bit of time reviewing the window functions patch,
and I think it is now ready to commit, other than the documentation
(which I've not looked at yet at all). Attached is my current patch
against HEAD, sans documentation. This incorporates the recently
2008/12/28 David Rowley dgrow...@gmail.com:
Tom Lane Wrote:
I've spent quite a bit of time reviewing the window functions patch,
and I think it is now ready to commit, other than the documentation
(which I've not looked at yet at all). Attached is my current patch
against HEAD, sans
David Rowley dgrow...@gmail.com writes:
I've started running my test queries that I used when reviewing the patch.
The following crashes the backend:
Fixed, thanks. (begin_partition was failing to reset spooled_rows when
falling out early because of empty outerplan; which could only cause an
Hitoshi Harada umi.tan...@gmail.com writes:
I ran the patch witouht any errors. Though it's trivial, I noticed
window_gettupleslot has to be fixed a bit.
Yeah, it could uselessly spool the partition before failing. I think
it had been that way before and I left it alone, but changing it is
Hitoshi Harada wrote:
WITH RECURSIVE bom AS (
SELECT parentpart,childpart,quantity,ROW_NUMBER() OVER (ORDER BY
parentpart DESC) rn
FROM billofmaterials
WHERE parentpart = 'KITCHEN'
UNION ALL
SELECT b.parentpart,b.childpart,b.quantity,ROW_NUMBER() OVER (ORDER BY
parentpart ASC)
Hitoshi Harada umi.tan...@gmail.com writes:
2008/12/28 David Rowley dgrow...@gmail.com:
I've started running my test queries that I used when reviewing the patch.
The following crashes the backend:
It seems that parseCheckWindowFuncs() doesn't check CTE case whereas
parseCheckAggregates()
... and it's committed. Congratulations!
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2008/12/29 Tom Lane t...@sss.pgh.pa.us:
... and it's committed. Congratulations!
regards, tom lane
Great! I am really glad I can contribute PostgreSQL project by such a
big improvement.
And I really thank all the hackers, without all the helps by you it
wouldn't be
2008/12/28 Tom Lane t...@sss.pgh.pa.us:
I've spent quite a bit of time reviewing the window functions patch,
and I think it is now ready to commit, other than the documentation
(which I've not looked at yet at all). Attached is my current patch
against HEAD, sans documentation. This
Hitoshi Harada umi.tan...@gmail.com writes:
2008/12/20 Tom Lane t...@sss.pgh.pa.us:
I've been studying the grammar for the windowing patch a bit. It seems
to me that the existing window name option for window specification
got left out.
I completely missed this issue. If the existing window
I wrote:
I've been hacking on this and I have a grammar that pretty much works,
but there's some bizarreness around UNBOUNDED. I'll post it later.
Here is a proof-of-concept grammar patch that allows frame_bound to use
a_expr instead of a hacked-up constant production (which, as I
complained
2008/12/20 Tom Lane t...@sss.pgh.pa.us:
Hitoshi Harada umi.tan...@gmail.com writes:
Here's the patch, including latest function default args. Regards,
The comments added to planner.c contain various claims that setrefs.c
will fix up window function references, but I see no code related to
2008/12/20 Tom Lane t...@sss.pgh.pa.us:
I've been studying the grammar for the windowing patch a bit. It seems
to me that the existing window name option for window specification
got left out. I think that WindowDef needs to have two name fields,
one for the name (if any) defined by the
2008/12/21 Hitoshi Harada umi.tan...@gmail.com:
2008/12/20 Tom Lane t...@sss.pgh.pa.us:
I've been studying the grammar for the windowing patch a bit. It seems
to me that the existing window name option for window specification
got left out. I think that WindowDef needs to have two name
Hitoshi Harada umi.tan...@gmail.com writes:
Here's the patch, including latest function default args. Regards,
The comments added to planner.c contain various claims that setrefs.c
will fix up window function references, but I see no code related to
that in setrefs.c. Please explain.
I've been studying the grammar for the windowing patch a bit. It seems
to me that the existing window name option for window specification
got left out. I think that WindowDef needs to have two name fields,
one for the name (if any) defined by the window definition, and one
for the referenced
2008/12/8 Heikki Linnakangas heikki.linnakan...@enterprisedb.com:
That said, we should try to get this committed ASAP, so I think we can live
without the trimming for 8.4.
Just let me know. What is the current status... Is there something for
me to do now? Or only wating?
Regards,
--
2008/12/6 David Rowley [EMAIL PROTECTED]:
I've spent last night and tonight trying to break the patch and I've not
managed it.
I spent 2 and a half hours on the train last night reading over the patch
mainly for my own interest. I also went over the documentation and I have a
few
2008/12/6 David Rowley [EMAIL PROTECTED]:
Hitoshi Harada Wrote:
2008/12/3 Hitoshi Harada [EMAIL PROTECTED]:
I am randomly trying some issues instead of agg common code (which I
now doubt if it's worth sharing the code), so tell me if you're
restarting your hack again. I'll send the whole
2008/12/7 Hitoshi Harada [EMAIL PROTECTED]:
2008/12/7 Hitoshi Harada [EMAIL PROTECTED]:
2008/12/6 David Rowley [EMAIL PROTECTED]:
the time where the community can test further by committing this patch.
Agree. I'll send the latest patch and finish my work for now.
Here's the patch, including
Hitoshi Harada wrote:
It shows even though tuplestore trims its tuples and stays in memory
rather than dumps them on files, the performance up is only 2 sec in
50 sec. So I concluded the optimization for row_number()/rank() etc
doesn't pay for its more complexity in window function API. The
Hitoshi Harada wrote:
I tested the spool performance with David's earlier bigtable:
CREATE TABLE bigtable (
id SERIAL NOT NULL PRIMARY KEY,
timestamp TIMESTAMP NOT NULL
);
-- about 383MB of data
INSERT INTO bigtable (timestamp)
SELECT NOW() + (CAST(RANDOM() * 10 AS INT) || '
Hitoshi Harada Wrote:
2008/12/3 Hitoshi Harada [EMAIL PROTECTED]:
I am randomly trying some issues instead of agg common code (which I
now doubt if it's worth sharing the code), so tell me if you're
restarting your hack again. I'll send the whole patch.
Attached is the updated patch,
2008/12/2 Hitoshi Harada [EMAIL PROTECTED]:
sample=# EXPLAIN ANALYZE SELECT LEAD(timestamp) OVER (ORDER BY id)
FROM bigtable LIMIT 1;
QUERY PLAN
--
---
2008/11/26 Heikki Linnakangas [EMAIL PROTECTED]:
Hitoshi Harada wrote:
I read more, and your spooling approach seems flexible for both now
and the furture. Looking at only current release, the frame with ORDER
BY is done by detecting peers in WinFrameGetArg() and add row number
of peers to
-Original Message-
From: Heikki Linnakangas [mailto:[EMAIL PROTECTED]
Sent: 26 November 2008 09:09
To: Hitoshi Harada
Cc: David Rowley; pgsql-hackers@postgresql.org
Subject: Re: Windowing Function Patch Review - Standard Conformance
Hitoshi Harada wrote:
2008/11/26 David Rowley
I wrote:
I was also reading over the standard tonight. I've discovered that the
OFFSET in LEAD() and LAG() is optional. It should default to 1 if it is
not present. Oracle seems to support this.
SQL2008 says:
If lead or lag function is specified, then:
i) Let VE1 be lead or lag extent
I wrote:
Hmm, did you apply the latest patch correctly? My build can produce
right results, so I don't see why it isn't fixed. Make sure the lines
around 2420-2430 in planner.c like:
/*
* must copyObject() to avoid args concatenating
with
David Rowley wrote:
I've created a query that uses the table in your regression test.
max_salary1 gives incorrect results. If you remove the max_salary2 column it
gives the correct results.
Thanks. I saw this myself yesterday, while hacking on the patch. I
thought it was a bug I had
2008/11/26 Heikki Linnakangas [EMAIL PROTECTED]:
Hitoshi Harada wrote:
2008/11/26 David Rowley [EMAIL PROTECTED]:
I'm at a bit of a loss to what to do now. Should I wait for your work
Heikki? Or continue validating this patch?
The best thing I can think to do right now is continue and any
2008/11/26 Heikki Linnakangas [EMAIL PROTECTED]:
Hitoshi Harada wrote:
I read more, and your spooling approach seems flexible for both now
and the furture. Looking at only current release, the frame with ORDER
BY is done by detecting peers in WinFrameGetArg() and add row number
of peers to
Hitoshi Harada wrote:
I read more, and your spooling approach seems flexible for both now
and the furture. Looking at only current release, the frame with ORDER
BY is done by detecting peers in WinFrameGetArg() and add row number
of peers to winobj-currentpos. Actually if we have capability to
Heikki Linnakangas [EMAIL PROTECTED] writes:
Here's another updated patch, including all your bug fixes.
I did a very fast pass through this and have a few comments.
* Don't bother manually updating keywords.sgml. As stated therein, that
table is automatically generated. All you'll
On 26/11/2008, Hitoshi Harada [EMAIL PROTECTED] wrote:
2008/11/26 David Rowley [EMAIL PROTECTED]:
Hitoshi Harada wrote:
2008/11/20 David Rowley [EMAIL PROTECTED]:
-- The following query gives incorrect results on the
-- maxhighbid column
SELECT auctionid,
category,
2008/11/27 Tom Lane [EMAIL PROTECTED]:
Heikki Linnakangas [EMAIL PROTECTED] writes:
Here's another updated patch, including all your bug fixes.
I did a very fast pass through this and have a few comments.
Thanks for your comments. The executor part is now being refactored by
Heikki, but
Hitoshi Harada wrote:
2008/11/20 David Rowley [EMAIL PROTECTED]:
-- The following query gives incorrect results on the
-- maxhighbid column
SELECT auctionid,
category,
description,
highestbid,
reserve,
MAX(highestbid) OVER (ORDER BY auctionid) AS
2008/11/26 David Rowley [EMAIL PROTECTED]:
Hitoshi Harada wrote:
2008/11/20 David Rowley [EMAIL PROTECTED]:
-- The following query gives incorrect results on the
-- maxhighbid column
SELECT auctionid,
category,
description,
highestbid,
reserve,
2008/11/25 Hitoshi Harada [EMAIL PROTECTED]:
2008/11/25 Heikki Linnakangas [EMAIL PROTECTED]:
Here's an updated patch, where the rows are fetched on-demand.
Good! And I like the fetching args by number better. Let me take more
time to look in detail...
I read more, and your spooling approach
Hitoshi Harada wrote:
2008/11/20 David Rowley [EMAIL PROTECTED]:
I won't be around until Monday evening (Tuesday morning JST). I'll pickup
the review again there. It's really time for me to push on with this review.
The patch is getting closer to getting the thumbs up from me. I'm really
hoping
2008/11/24 Heikki Linnakangas [EMAIL PROTECTED]:
Hitoshi Harada wrote:
2008/11/20 David Rowley [EMAIL PROTECTED]:
I won't be around until Monday evening (Tuesday morning JST). I'll pickup
the review again there. It's really time for me to push on with this
review.
The patch is getting
2008/11/25 Heikki Linnakangas [EMAIL PROTECTED]:
Hitoshi Harada wrote:
If you keep this in your mind, please don't be annoyed but the current
frame concept is wrong.
...
Note that on empno=4 then last_value=4(yours)/3(mine), which means the
frame is applied to last_value() as well as
I wrote:
All,
This is my first patch review for PostgreSQL. I did submit a patch last
commit fest (Boyer-Moore) so I feel I should review one this commit fest.
I'm quite new to PostgreSQL so please don't rely on me totally. I'll do my
best. Heikki is also reviewing this patch which makes me
2008/11/9 David Rowley [EMAIL PROTECTED]:
Hitoshi Harada wrote:
I recreate the patch against current HEAD, in the git it's here:
http://git.postgresql.org/?p=postgresql.git;a=commit;h=f88970d3c6fb9f99543
d873bb7228f4c057c23e0
I tested `patch -p1` with the attached and succeeded to make it
Using one of my original test tables I'm testing windowing functions with a
GROUP BY.
The following query works as I would expect.
-- Works
SELECT department,
SUM(Salary),
ROW_NUMBER() OVER (ORDER BY department),
SUM(SUM(salary)) OVER (ORDER BY department)
FROM employees
Hitoshi Harada wrote:
I'm glad to hear that. Actually thanks to git it is quite easy for me
to merge my own repository with the HEAD. It tells me which lines are
new coming and which lines I modified are newer than else in CVS. This
is my first project where I use git (and I am not guru of cvs
Hitoshi Harada wrote:
I recreate the patch against current HEAD, in the git it's here:
http://git.postgresql.org/?p=postgresql.git;a=commit;h=f88970d3c6fb9f99543
d873bb7228f4c057c23e0
I tested `patch -p1` with the attached and succeeded to make it work
cleanly. It seems to me that this
Hitoshi Harada Wrote:
although attached is the whole (split) patch.
I'm having some trouble getting these patches to patch cleanly. I think it's
because of this that I can't get postgres to compile after applying the
patch.
It errors out at tuptoaster.c some constants seem to be missing from
2008/11/9 David Rowley [EMAIL PROTECTED]:
Hitoshi Harada Wrote:
although attached is the whole (split) patch.
I'm having some trouble getting these patches to patch cleanly. I think it's
because of this that I can't get postgres to compile after applying the
patch.
It errors out at
David Rowley [EMAIL PROTECTED] writes:
patching file src/include/catalog/pg_proc.h
Hunk #4 FAILED at 106.
1 out of 4 hunks FAILED -- saving rejects to file
src/include/catalog/pg_proc.h.rej
I imagine you'll find that hunk #4 covers the entire DATA() body of
the file :-(. It can't possibly
Instead of a patch it might be easier to submit the new columns as a
perl script or sed command. We do something like that to make merging
pg_proc easier.
greg
On 8 Nov 2008, at 01:36 PM, Tom Lane [EMAIL PROTECTED] wrote:
David Rowley [EMAIL PROTECTED] writes:
patching file
I wrote:
All,
This is my first patch review for PostgreSQL. I did submit a patch last
commit fest (Boyer-Moore) so I feel I should review one this commit fest.
I'm quite new to PostgreSQL so please don't rely on me totally. I'll do my
best. Heikki is also reviewing this patch which makes me
Quoted from SQL:2008
If CUME_DIST is specified, then the relative *rank *of a row R is defined
as
NP/NR, where NP is defined
to be the number of rows preceding or peer with R in the window ordering of
the window partition of R
and NR is defined to be the number of rows in the window
2008/11/5 Vladimir Sitnikov [EMAIL PROTECTED]:
Quoted from SQL:2008
If CUME_DIST is specified, then the relative rank of a row R is defined
as
NP/NR, where NP is defined
to be the number of rows preceding or peer with R in the window ordering
of
the window partition of R
and NR is defined
Even though I understand the definition, your suggestion of COUNT(*)
OVER (ORDER BY salary) doesn't make sense.
Why does not that make sense?
I have not read the spec, however Oracle has a default window specification
in case there is only an order by clause. The default window is range
2008/11/5 Vladimir Sitnikov [EMAIL PROTECTED]:
Even though I understand the definition, your suggestion of COUNT(*)
OVER (ORDER BY salary) doesn't make sense.
Why does not that make sense?
I have not read the spec, however Oracle has a default window specification
in case there is only an
58 matches
Mail list logo
