Kyotaro HORIGUCHI 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 but the
> new patch of course
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 GROU
Kyotaro HORIGUCHI 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 columns of the primary key.
In
# 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 discu
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
(2014/04/10 22:25), Tom Lane wrote:
> Etsuro Fujita 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 col
Etsuro Fujita 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 following code in in
(2014/04/10 0:08), Tom Lane wrote:
> Kyotaro HORIGUCHI 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 thi
Kyotaro HORIGUCHI 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 get the wrong answer. It looks
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
> > wrote:
> > > May I mark this patch as "Ready for Committer" by myself since
> >
Hello,
At Tue, 25 Feb 2014 16:35:55 -0500, Robert Haas wrote
> On Tue, Feb 25, 2014 at 3:36 AM, Kyotaro HORIGUCHI
> 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
On Tue, Feb 25, 2014 at 3:36 AM, Kyotaro HORIGUCHI
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 Haas
EnterpriseDB: http://w
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 pat
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,
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 t
Kyotaro HORIGUCHI 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
> +++ b/src/backend/optimizer/path/path
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 (thoug
> 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 gen
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 reins
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
"Etsuro Fujita" 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 incorrect join pathkeys', w
Tom Lane wrote:
> "Etsuro Fujita" 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 sufficie
"Etsuro Fujita" 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 sufficient to check that
the index
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 (pgsql-hacker
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 lis
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
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.
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
> pr
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 tw
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
pathkey_and_uniqueindx_v7_20
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,
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
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)
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 pgsql-hacke
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
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 incor
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 accordin
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 attche
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 non
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 patc
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
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 subscri
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
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
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 ass
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:
http://pgci.eisentraut.org/jenkins/job/postgresql_commitfest_world/175/artifact/src/test/isolation/regression
I wrote:
> Kyotaro HORIGUCHI 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 optimization(s)
to cases
On Thu, Oct 31, 2013 at 10:59 AM, Tom Lane 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 a separate pat
Kyotaro HORIGUCHI 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=# insert into t
>> (select a
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);
> uni
52 matches
Mail list logo
