Hello,
I thought some more about this patch, and realized that it's more or less
morally equivalent to allowing references to ungrouped variables when the
query has a GROUP BY clause listing all the columns of the primary key.
In that case the parser is effectively pretending that the GROUP
Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes:
Ok, I am convinced that your suggestion - truncating
query_pathkeys by removing eventually no-op entries - seems
preferable and will have wider effect naturally - more promised.
I won't persist with the way this patch currently does
Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes:
[ pathkey_and_uniqueindx_v10_20130411.patch ]
I thought some more about this patch, and realized that it's more or less
morally equivalent to allowing references to ungrouped variables when the
query has a GROUP BY clause listing all the
(2014/04/10 0:08), Tom Lane wrote:
Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes:
Oops! I found a bug in this patch. The previous v8 patch missed
the case that build_index_pathkeys() could build a partial
pathkeys from the index tlist.
TBH I think that's barely the tip of the
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:
(2014/04/10 0:08), Tom Lane wrote:
TBH I think that's barely the tip of the iceberg of cases where this
patch will get the wrong answer.
Also, I don't see it doing anything to check the ordering
of multiple index columns
I think that the
(2014/04/10 22:25), Tom Lane wrote:
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:
(2014/04/10 0:08), Tom Lane wrote:
TBH I think that's barely the tip of the iceberg of cases where this
patch will get the wrong answer.
Also, I don't see it doing anything to check the ordering
of
Hi, sorry for the absense. I've been back.
Attached is the patch following the discussion below.
(2014/04/10 0:08), Tom Lane wrote:
TBH I think that's barely the tip of the iceberg of cases where this
patch will get the wrong answer.
Also, I don't see it doing anything to check the
# Sorry for accidentialy sending the previous mail unfinished.
## ...and I seem to have bombed uncertain files off out of my
## home directory by accident, too :(
=
Hi, sorry for the absense. I've been back.
Thank you for continuing this discussion.
Attached is the patch following the
Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes:
Oops! I found a bug in this patch. The previous v8 patch missed
the case that build_index_pathkeys() could build a partial
pathkeys from the index tlist.
TBH I think that's barely the tip of the iceberg of cases where this
patch will
Hi Horiguchi-san,
Sorry for not reviewing this patch in the last CF.
(2014/03/10 16:21), Kyotaro HORIGUCHI wrote:
Oops! I found a bug in this patch. The previous v8 patch missed
the case that build_index_pathkeys() could build a partial
pathkeys from the index tlist.
This causes the situation
Oops! I found a bug in this patch. The previous v8 patch missed
the case that build_index_pathkeys() could build a partial
pathkeys from the index tlist.
This causes the situation follows,
===
=# \d cu11
Table public.cu11
Column | Type | Modifiers
+-+---
a
I marked this patch as 'Ready for Committer' by myself according
to the following discussion.
Thanks.
At Tue, 25 Feb 2014 16:35:55 -0500, Robert Haas wrote
On Tue, Feb 25, 2014 at 3:36 AM, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:
May I mark this patch as Ready for
Hello.
May I mark this patch as Ready for Committer by myself since
this was already marked so in last CF3 and have had no objection
or new suggestion in this CF4 for more than a month?
At Tue, 14 Jan 2014 18:08:10 +0900, Kyotaro HORIGUCHI wrote
Hello, since CF4 is already closed but this patch
On Tue, Feb 25, 2014 at 3:36 AM, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:
May I mark this patch as Ready for Committer by myself since
this was already marked so in last CF3 and have had no objection
or new suggestion in this CF4 for more than a month?
Sounds fair.
--
Robert
Hello,
At Tue, 25 Feb 2014 16:35:55 -0500, Robert Haas wrote
On Tue, Feb 25, 2014 at 3:36 AM, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:
May I mark this patch as Ready for Committer by myself since
this was already marked so in last CF3 and have had no objection
or new
Hello, since CF4 is already closed but this patch ramains marked
as 'Ready for Committer', please let me re-post the latest
version for CF4 to get rid of vanishing :-p
tgl But aside from hasty typos,
Oops! I've picked up wrong node. It always denies pathkeys extension.
| !IsA(member, Var))
Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes:
The following modification to v7 does this.
=
diff --git a/src/backend/optimizer/path/pathkeys.c
b/src/backend/optimizer/path/pathkeys.c
index 380f3ba..233e21c 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++
Hello,
tgl I'm pretty sure that IsA test prevents the optimization from ever firing.
Thank you.
tgl But aside from hasty typos,
Oops! I've picked up wrong node. It always denies pathkeys extension.
| !IsA(member, Var))
is a mistake of the following.
| !IsA(member-em_expr, Var))
tgl is
Tom Lane wrote:
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:
[ pathkey_and_uniqueindx_v7_20131203.patch ]
I started to look at this patch. I don't understand the reason for the
foreach loop in index_pathkeys_are_extensible (and the complete lack of
comments in the patch isn't
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:
Thank you for taking time to look at this patch. I think it's not
sufficient to check those things. Let me explain the reason why this patch
has that code. The patch has that code in order to prevent
build_join_pathkeys() from building
Hello,
Tom Lane wrote:
I started to look at this patch. I don't understand the reason for the
foreach loop in index_pathkeys_are_extensible (and the complete lack of
comments in the patch isn't helping). Isn't it sufficient to check that
the index is unique/immediate/allnotnull and its
Hello,
tgl The problem is that joining isn't the only way that such expansion can
tgl happen. Set-returning functions in the targetlist are another way,
tgl and I'm not sure that there aren't others. Here's an example that
tgl I'm pretty sure breaks your patch (though I didn't actually
I think that the required condition for all these ordering
Careless wording. the 'required' condition above means 'necessary
(but not sufficient)' condition so I think the lack of the
precondition (=multiple rows) results in prevention of the
postcondition = ordering problems.
problems is
Kyotaro HORIGUCHI wrote:
tgl The problem is that joining isn't the only way that such expansion
tgl can happen. Set-returning functions in the targetlist are another
tgl way, and I'm not sure that there aren't others. Here's an example
tgl that I'm pretty sure breaks your patch (though I
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes:
[ pathkey_and_uniqueindx_v7_20131203.patch ]
I started to look at this patch. I don't understand the reason for the
foreach loop in index_pathkeys_are_extensible (and the complete lack of
comments in the patch isn't helping). Isn't it
I wrote:
Kyotaro HORIGUCHI wrote:
I'm convinced of the validity of your patch. I'm satisfied with it.
Then, if there are no objections of others, I'll mark this as Ready for
Committer.
Done.
Thanks,
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list
Thank you,
One is, you put the added code for getrelation_info() out of the block for
the condition (info-relam == BTREE_AM_OID) (though amcanorder would be
..
By checking the following equation in build_index_paths(), the updated
version of the patch guarantees that the result of an index
Kyotaro HORIGUCHI wrote:
I'm convinced of the validity of your patch. I'm satisfied with it. Thank
you.
Thank you for the reply.
Then, if there are no objections of others, I'll mark this as Ready for
Committer.
Thanks,
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list
Hello,
I've modified the patch to work in such a way. Also, as ISTM the patch
is more complicated than what the patch really does, I've simplified the
patch.
I've revised the patch a bit. Please find attached the patch.
Thank you, but it seems to me too simplified. You made two major
Kyotaro HORIGUCHI wrote:
Thank you, but it seems to me too simplified. You made two major
functional
changes.
Thank you for the comments!
One is, you put the added code for getrelation_info() out of the block for
the condition (info-relam == BTREE_AM_OID) (though amcanorder would be
I wrote:
Kyotaro HORIGUCHI wrote:
Another is, you changed pathkeys expantion to be all-or-nothing
decision.
While this change should simplify the code slightly, it also dismisses
the oppotunity for partially-extended pathkeys. Could you let me know
the
reason
why you did so.
At first
I wrote:
I've modified the patch to work in such a way. Also, as ISTM the patch
is more complicated than what the patch really does, I've simplified the
patch.
I've revised the patch a bit. Please find attached the patch.
Thanks,
Best regards,
Etsuro Fujita
I wrote:
I wrote:
Kyotaro HORIGUCHI wrote:
* In get_relation_info(), the patch determines the branch
condition keyattno != ObjectIdAttributeNumber. I fail to
understand the meaning of this branch condition. Could you explain
about it?
Literally answering, it means oid cannot
Kyotaro HORIGUCHI wrote:
* In get_relation_info(), the patch determines the branch condition
keyattno != ObjectIdAttributeNumber. I fail to understand the
meaning of this branch condition. Could you explain about it?
Literally answering, it means oid cannot be NULL (if it exists).
I wrote:
Kyotaro HORIGUCHI wrote:
* In get_relation_info(), the patch determines the branch condition
keyattno != ObjectIdAttributeNumber. I fail to understand the
meaning of this branch condition. Could you explain about it?
Literally answering, it means oid cannot be NULL (if it
Thank you for pointing out.
the attched pathkey_and_uniqueindx_v4_20131122.patch is changed as
follows.
The patch is compiled successfully and passes all regression tests. Also,
the patch works well for the tests shown in an earlier email from
Horiguchi-san. But in this version I found
On Fri, 2013-11-22 at 16:59 +0900, Kyotaro HORIGUCHI wrote:
Hello. I found a bug(?) in thsi patch as I considered on another
patch.
src/backend/optimizer/util/plancat.c:256: trailing whitespace.
src/backend/optimizer/util/plancat.c:261: space before tab in indent.
--
Sent via
Kyotaro HORIGUCHI wrote:
the attched pathkey_and_uniqueindx_v4_20131122.patch is changed as
follows.
The patch is compiled successfully and passes all regression tests. Also,
the patch works well for the tests shown in an earlier email from
Horiguchi-san. But in this version I found an
Hello. I found a bug(?) in thsi patch as I considered on another
patch.
In truncate_useless_pathkeys, if query_pathkeys is longer than
pathkeys made from index columns old patch picked up the latter
as IndexPath's pathkeys. But the former is more suitable
according to the context here.
the
Kyotaro HORIGUCHI wrote:
Hello. I found a bug(?) in thsi patch as I considered on another patch.
In truncate_useless_pathkeys, if query_pathkeys is longer than pathkeys
made
from index columns old patch picked up the latter as IndexPath's pathkeys.
But the former is more suitable according
Kyotaro HORIGUCHI wrote:
Hello, I've totally refactored the series of patches and cut out the
appropriate portion as 'unique (and non-nullable) index stuff'.
As the discussion before, it got rid of path distinctness. This patch
works only on index 'full-orederedness', i.e., unique index on
Hello, I've totally refactored the series of patches and cut out
the appropriate portion as 'unique (and non-nullable) index
stuff'.
As the discussion before, it got rid of path distinctness. This
patch works only on index 'full-orederedness', i.e., unique index
on non-nullable columns.
This
Hello, I might made insufficient explanation. Surely it is
overdone for the example.
uniquetest=# create table t (a int, b int, c int, d text);
uniquetest=# create unique index i_t_pkey on t(a, b);
uniquetest=# insert into t
(select a % 10, a / 10, a, 't' from generate_series(0, 10) a);
On Tue, 2013-11-12 at 17:48 +0900, Kyotaro HORIGUCHI wrote:
Hello, this is the revised patch.
Since you're using git, please check your patch for trailing whitespace
with git diff --check.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your
Hello, this is the revised patch.
Hmm, that sounds quite resonable in general. But the conflation
is already found in grouping_planner to some extent.
Although this new patch is not split into unique-index sutff and
others, it removes current_pathkeys from grouping_planner, and
adds pathkeys
Thank you,
In any case, it seems like a bad idea to me to conflate
distinct-ness with ordering, so I don't like what you did to
PathKeys.
Hmm, that sounds quite resonable in general. But the conflation
is already found in grouping_planner to some extent. The name
distinct_pathkey itself
Hello,
Your patch fails the isolation test because of changed query plans:
http://pgci.eisentraut.org/jenkins/job/postgresql_commitfest_world/175/artifact/src/test/isolation/regression.diffs/*view*/
Thank you for pointing out. I wasn't aware of that..
# Because it is not launched from the
On 10/31/13, 6:43 AM, Kyotaro HORIGUCHI wrote:
Hello, This is the last episode of the 'dance with indices'?
series.
Your patch fails the isolation test because of changed query plans:
Hello, This is the last episode of the 'dance with indices'?
series.
Unique indexes can sort the tuples in corresponding tables
prefectly. So this query might can use index.
uniquetest=# create table t (a int, b int, c int, d text);
uniquetest=# create unique index i_t_pkey on t(a, b);
Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes:
Unique indexes can sort the tuples in corresponding tables
prefectly. So this query might can use index.
uniquetest=# create table t (a int, b int, c int, d text);
uniquetest=# create unique index i_t_pkey on t(a, b);
uniquetest=#
On Thu, Oct 31, 2013 at 10:59 AM, Tom Lane t...@sss.pgh.pa.us wrote:
However, if the index is unique, wouldn't
scanning the index produce data that actually satisfies the longer sort
key? It doesn't matter what the values of c,d are if there are no
duplicates in the a,b columns. So maybe as
I wrote:
Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes:
Unique indexes can sort the tuples in corresponding tables
prefectly. So this query might can use index.
BTW, how much of any of this is correct if the unique index contains
multiple NULL values? We might need to restrict the
52 matches
Mail list logo