On Fri, Apr 03, 2020 at 03:54:38PM +0900, Michael Paquier wrote:
> Now, would it be better to apply the refactoring patch for HEAD before
> feature freeze, or are people fine if this is committed a bit after?
> Patch 0002 is neither a new feature, nor an actual bug, and just some
> code cleanup,
On Thu, Apr 02, 2020 at 04:38:36AM -0300, Alvaro Herrera wrote:
> I don't think we need to worry about that problem, because we already
> checked that the pg_class tuple for the index is there two lines above.
> The pg_index tuple cannot have gone away on its own; and the index can't
> be deleted
On Thu, Apr 02, 2020 at 04:24:03PM +0900, Michael Paquier wrote:
> On Thu, Apr 02, 2020 at 01:52:09AM -0500, Justin Pryzby wrote:
>> Sounds right. Or else get_index_isclustered() could be redefined to take a
>> boolean "do_elog" flag, and if syscache fails and that's false, then return
>> false
On 2020-Apr-02, Michael Paquier wrote:
> Now, regarding patch 0002, note that you have a problem for this part:
> -tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexOid));
> -if (!HeapTupleIsValid(tuple))/* probably can't happen */
> -{
> -
On Thu, Apr 02, 2020 at 01:52:09AM -0500, Justin Pryzby wrote:
> Sounds right. Or else get_index_isclustered() could be redefined to take a
> boolean "do_elog" flag, and if syscache fails and that's false, then return
> false instead of ERROR.
Not sure if that's completely right to do either.
On Thu, Apr 02, 2020 at 03:14:21PM +0900, Michael Paquier wrote:
> Now, regarding patch 0002, note that you have a problem for this part:
> -tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexOid));
> -if (!HeapTupleIsValid(tuple))/* probably can't happen */
> -
On Wed, Mar 18, 2020 at 11:48:37AM +0900, Michael Paquier wrote:
> Anyway, Tom, Alvaro, are you planning to look at what is proposed on
> this thread? I don't want to step on your toes if that's the case and
> it seems to me that the approach taken by the patch is sound, using as
> basic fix the
On Tue, Mar 17, 2020 at 11:20:44AM -0500, Justin Pryzby wrote:
> On Tue, Mar 17, 2020 at 02:33:32PM +0900, Michael Paquier wrote:
>> Patch 0002 from Justin does that, I would keep this refactoring as
>> HEAD-only material though, and I don't spot any other code paths in
>> need of patching.
>>
>>
On Tue, Mar 17, 2020 at 02:33:32PM +0900, Michael Paquier wrote:
> > Yeah, in cluster(), mark_index_clustered().
>
> Patch 0002 from Justin does that, I would keep this refactoring as
> HEAD-only material though, and I don't spot any other code paths in
> need of patching.
>
> The commit message
On Mon, Mar 16, 2020 at 11:25:23AM -0300, Alvaro Herrera wrote:
> On 2020-Mar-16, Justin Pryzby wrote:
>
> > Also, should we call it "is_index_clustered", since otherwise it sounds alot
> > like "+get_index_clustered" (without "is"), which sounds like it takes a
> > table
> > and returns which
On 2020-Mar-16, Justin Pryzby wrote:
> Also, should we call it "is_index_clustered", since otherwise it sounds alot
> like "+get_index_clustered" (without "is"), which sounds like it takes a table
> and returns which index is clustered. That might be just as useful for some
> of
> these
be just as useful for some of
these callers.
--
Justin
>From add5e8481c70b6b66342b264a243f26f4c634e53 Mon Sep 17 00:00:00 2001
From: Amit Langote
Date: Mon, 16 Mar 2020 16:01:42 +0900
Subject: [PATCH v7 1/2] ALTER tbl rewrite loses CLUSTER ON index
On Fri, Mar 13, 2020 at 2:19 AM Tom Lane wrote:
> Justin Pryzby writ
On Fri, Mar 13, 2020 at 2:19 AM Tom Lane wrote:
> Justin Pryzby writes:
> > @cfbot: resending with only Amit's 0001, since Michael pushed a variation on
> > 0002.
Thank you for taking a look at it.
> Boy, I really dislike this patch. ATPostAlterTypeParse is documented as
> using the supplied
Justin Pryzby writes:
> @cfbot: resending with only Amit's 0001, since Michael pushed a variation on
> 0002.
Boy, I really dislike this patch. ATPostAlterTypeParse is documented as
using the supplied definition string, and nothing else, to reconstruct
the index. This breaks that without even
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
I tested the patch on the master branch
@cfbot: resending with only Amit's 0001, since Michael pushed a variation on
0002.
--
Justin
>From 865fc2713ad29d0f8c0f63609a7c15c83cfa5cfe Mon Sep 17 00:00:00 2001
From: Amit Langote
Date: Thu, 6 Feb 2020 18:14:16 +0900
Subject: [PATCH v5] ALTER tbl rewrite loses CLUSTER ON index
---
On Mon, Mar 02, 2020 at 12:28:18PM +0900, Michael Paquier wrote:
> > +SELECT indexrelid::regclass FROM pg_index WHERE
> > indrelid='concur_clustered'::regclass;
>
> This test should check after indisclustered. Except that, the patch
> is fine so I'll apply it if there are no objections.
Oops -
On Mon, Mar 02, 2020 at 12:28:18PM +0900, Michael Paquier wrote:
> This test should check after indisclustered. Except that, the patch
> is fine so I'll apply it if there are no objections.
I got a second look at this one, and applied it down to 12 after some
small modifications in the new test
On Sat, Feb 29, 2020 at 10:52:58AM -0600, Justin Pryzby wrote:
> Rebased Amit's patch and included my own 0002 to fix the REINDEX CONCURRENTLY
> issue.
I have looked at 0002 as that concerns me.
> +SELECT indexrelid::regclass FROM pg_index WHERE
> indrelid='concur_clustered'::regclass;
> +
ut CONCURRENTLY there's
> no
> issue. I guess we need a separate patch for that case.
Rebased Amit's patch and included my own 0002 to fix the REINDEX CONCURRENTLY
issue.
--
Justin
>From dccc6f08450eacafc3a08bc8b2836d2feb23a533 Mon Sep 17 00:00:00 2001
From: Amit Langote
Date
On Fri, Feb 28, 2020 at 06:26:04PM -0500, Tom Lane wrote:
> Justin Pryzby writes:
> > I think the attached is 80% complete (I didn't touch pg_dump).
> > One objection to this change would be that all relations (including indices)
> > end up with relclustered fields, and pg_index already has a
Justin Pryzby writes:
> I think the attached is 80% complete (I didn't touch pg_dump).
> One objection to this change would be that all relations (including indices)
> end up with relclustered fields, and pg_index already has a number of bools,
> so
> it's not like this one bool is wasting a
On Mon, Feb 17, 2020 at 02:31:42PM +0900, Amit Langote wrote:
> Hi Justin,
>
> On Fri, Feb 7, 2020 at 11:39 PM Justin Pryzby wrote:
> > On Thu, Feb 06, 2020 at 02:24:47PM -0300, Alvaro Herrera wrote:
> > > On 2020-Feb-06, Justin Pryzby wrote:
> > >
> > > > I wondered if it wouldn't be better if
Hi Justin,
On Fri, Feb 7, 2020 at 11:39 PM Justin Pryzby wrote:
> On Thu, Feb 06, 2020 at 02:24:47PM -0300, Alvaro Herrera wrote:
> > On 2020-Feb-06, Justin Pryzby wrote:
> >
> > > I wondered if it wouldn't be better if CLUSTER ON was stored in pg_class
> > > as the
> > > Oid of a clustered
On Thu, Feb 06, 2020 at 02:24:47PM -0300, Alvaro Herrera wrote:
> On 2020-Feb-06, Justin Pryzby wrote:
>
> > I wondered if it wouldn't be better if CLUSTER ON was stored in pg_class as
> > the
> > Oid of a clustered index, rather than a boolean in pg_index.
>
> Maybe. Do you want to try a
On Fri, Feb 7, 2020 at 2:24 AM Alvaro Herrera wrote:
> On 2020-Feb-06, Justin Pryzby wrote:
> > I wondered if it wouldn't be better if CLUSTER ON was stored in pg_class as
> > the
> > Oid of a clustered index, rather than a boolean in pg_index.
>
> Maybe. Do you want to try a patch?
+1
On 2020-Feb-06, Justin Pryzby wrote:
> I wondered if it wouldn't be better if CLUSTER ON was stored in pg_class as
> the
> Oid of a clustered index, rather than a boolean in pg_index.
Maybe. Do you want to try a patch?
> That likely would've avoided (or at least exposed) this issue.
> And
I wondered if it wouldn't be better if CLUSTER ON was stored in pg_class as the
Oid of a clustered index, rather than a boolean in pg_index.
That likely would've avoided (or at least exposed) this issue.
And avoids the possibility of having two indices marked as "clustered".
These would be more
On Thu, Feb 6, 2020 at 10:31 AM Amit Langote wrote:
> On Thu, Feb 6, 2020 at 8:44 AM Justin Pryzby wrote:
> > On Wed, Feb 05, 2020 at 03:53:45PM +0900, Amit Langote wrote:
> > > diff --git a/src/test/regress/sql/alter_table.sql
> > > b/src/test/regress/sql/alter_table.sql
> > > +-- alter type
On Wed, Feb 05, 2020 at 03:53:45PM +0900, Amit Langote wrote:
> Hi Justin,
>
> On Mon, Feb 3, 2020 at 1:17 AM Justin Pryzby wrote:
> > Other options are preserved by ALTER (and CLUSTER ON is and most obviously
> > should be preserved by CLUSTER's rewrite), so I think (SET) CLUSTER should
> > be
Hi Justin,
On Mon, Feb 3, 2020 at 1:17 AM Justin Pryzby wrote:
> Other options are preserved by ALTER (and CLUSTER ON is and most obviously
> should be preserved by CLUSTER's rewrite), so I think (SET) CLUSTER should be
> preserved by ALTER, too.
Yes.
create table foo (a int primary key);
Other options are preserved by ALTER (and CLUSTER ON is and most obviously
should be preserved by CLUSTER's rewrite), so I think (SET) CLUSTER should be
preserved by ALTER, too.
As far as I can see, this should be the responsibility of something in the
vicinity of
32 matches
Mail list logo
