Re: CLUSTER patch and regression test

Hi:

I sent the CLUSTER patch a few days ago and I think it was missed. I
append it again, this time including the regression test files. For the
committer, please note that you have to cvs add the files as they don't
exist. Maybe add to the parallel and serial schedules also, but I don't
know such stuff.

If there are no objections, please apply.

--
Alvaro Herrera (<alvherre[a]atentus.com>)
"La primera ley de las demostraciones en vivo es: no trate de usar el sistema.
Escriba un guión que no toque nada para no causar daños." (Jakob Nielsen)

I thought Tom wanted to still review it so I waited. Let me add it to
the patch queue now and give him a day or two to check it.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Alvaro Herrera wrote:
> Hi:
>
> I sent the CLUSTER patch a few days ago and I think it was missed. I
> append it again, this time including the regression test files. For the
> committer, please note that you have to cvs add the files as they don't
> exist. Maybe add to the parallel and serial schedules also, but I don't
> know such stuff.
>
> If there are no objections, please apply.
>
> --
> Alvaro Herrera (<alvherre[a]atentus.com>)
> "La primera ley de las demostraciones en vivo es: no trate de usar el sistema.
> Escriba un gui?n que no toque nada para no causar da?os." (Jakob Nielsen)

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Alvaro Herrera <alvherre(at)atentus(dot)com> writes:
> I sent the CLUSTER patch a few days ago and I think it was missed. I
> append it again, this time including the regression test files.

Applying the patch to current CVS and compiling produces:

gcc -g3 -O0 -DPS_USE_NONE -Wall -Wmissing-prototypes -Wmissing-declarations -I../../../src/include -c -o cluster.o cluster.c -MMD
cluster.c: In function `swap_relfilenodes':
cluster.c:383: `Num_pg_class_indices' undeclared (first use in this function)
cluster.c:383: (Each undeclared identifier is reported only once
cluster.c:383: for each function it appears in.)
cluster.c:432: warning: implicit declaration of function `CatalogOpenIndices'
cluster.c:432: `Name_pg_class_indices' undeclared (first use in this function)
cluster.c:433: too many arguments to function `CatalogIndexInsert'
cluster.c:434: too many arguments to function `CatalogIndexInsert'
cluster.c:435: warning: implicit declaration of function `CatalogCloseIndices'
cluster.c:383: warning: unused variable `irels'
make[3]: *** [cluster.o] Error 1
make[3]: Leaving directory `/home/nconway/pgsql/src/backend/commands'

Cheers,

Neil

--
Neil Conway <neilconway(at)rogers(dot)com>
PGP Key ID: DB3C29FC

Neil Conway dijo:

> Alvaro Herrera <alvherre(at)atentus(dot)com> writes:
> > I sent the CLUSTER patch a few days ago and I think it was missed. I
> > append it again, this time including the regression test files.
>
> Applying the patch to current CVS and compiling produces:
>
> gcc -g3 -O0 -DPS_USE_NONE -Wall -Wmissing-prototypes -Wmissing-declarations -I../../../src/include -c -o cluster.o cluster.c -MMD
> cluster.c: In function `swap_relfilenodes':
> cluster.c:383: `Num_pg_class_indices' undeclared (first use in this function)

Doh. I think this relates to the recent changes Tom did to the routines
related to system indexes. I'll update and send a new version later.

--
Alvaro Herrera (<alvherre[a]atentus.com>)
"Es filosofo el que disfruta con los enigmas" (G. Coli)

Neil Conway <nconway(at)klamath(dot)dyndns(dot)org> writes:
> Alvaro Herrera <alvherre(at)atentus(dot)com> writes:
>> I sent the CLUSTER patch a few days ago and I think it was missed. I
>> append it again, this time including the regression test files.

> Applying the patch to current CVS and compiling produces:

> gcc -g3 -O0 -DPS_USE_NONE -Wall -Wmissing-prototypes -Wmissing-declarations -I../../../src/include -c -o cluster.o cluster.c -MMD
> cluster.c: In function `swap_relfilenodes':
> cluster.c:383: `Num_pg_class_indices' undeclared (first use in this function)
> cluster.c:383: (Each undeclared identifier is reported only once
> cluster.c:383: for each function it appears in.)
> cluster.c:432: warning: implicit declaration of function `CatalogOpenIndices'

Looks like Alvaro got sideswiped by the system catalog indexing changes
I made over the weekend. It's a simple change, just reduce the whole
mess to a "CatalogUpdateIndexes()" call.

regards, tom lane

Yeah - Alvaro needs to sync with Tom's catalog index improvements - he
should be able to see example code in tablecmds.c, or in the orginal version
of the file this error is in?

Chris

> Alvaro Herrera <alvherre(at)atentus(dot)com> writes:
> > I sent the CLUSTER patch a few days ago and I think it was missed. I
> > append it again, this time including the regression test files.
>
> Applying the patch to current CVS and compiling produces:
>
> gcc -g3 -O0 -DPS_USE_NONE -Wall -Wmissing-prototypes
> -Wmissing-declarations -I../../../src/include -c -o cluster.o
> cluster.c -MMD
> cluster.c: In function `swap_relfilenodes':
> cluster.c:383: `Num_pg_class_indices' undeclared (first use in
> this function)
> cluster.c:383: (Each undeclared identifier is reported only once
> cluster.c:383: for each function it appears in.)
> cluster.c:432: warning: implicit declaration of function
> `CatalogOpenIndices'
> cluster.c:432: `Name_pg_class_indices' undeclared (first use in
> this function)
> cluster.c:433: too many arguments to function `CatalogIndexInsert'
> cluster.c:434: too many arguments to function `CatalogIndexInsert'
> cluster.c:435: warning: implicit declaration of function
> `CatalogCloseIndices'
> cluster.c:383: warning: unused variable `irels'
> make[3]: *** [cluster.o] Error 1
> make[3]: Leaving directory `/home/nconway/pgsql/src/backend/commands'
>
> Cheers,
>
> Neil
>
> --
> Neil Conway <neilconway(at)rogers(dot)com>
> PGP Key ID: DB3C29FC
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>

Tom Lane dijo:

> Looks like Alvaro got sideswiped by the system catalog indexing changes
> I made over the weekend. It's a simple change, just reduce the whole
> mess to a "CatalogUpdateIndexes()" call.

I update two tuples, so I manually CatalogOpenIndexes() and
CatalogIndexInsert() two times, as per comments in
CatalogUpdateIndexes().

I also removed a couple of useless CommandCounterIncrement(), some
useless definitions in src/include/commands/cluster.h and useless
includes in src/backend/commands/cluster.c. This version passes the
regression test I had made for previous versions.

If it applies cleanly, please apply :-)

--
Alvaro Herrera (<alvherre[a]atentus.com>)
La web junta la gente porque no importa que clase de mutante sexual seas,
tienes millones de posibles parejas. Pon "buscar gente que tengan sexo con
ciervos incendiánse", y el computador dirá "especifique el tipo de ciervo"
(Jason Alexander)

Patch applied. Thanks.

---------------------------------------------------------------------------

Alvaro Herrera wrote:
> Hi:
>
> I sent the CLUSTER patch a few days ago and I think it was missed. I
> append it again, this time including the regression test files. For the
> committer, please note that you have to cvs add the files as they don't
> exist. Maybe add to the parallel and serial schedules also, but I don't
> know such stuff.
>
> If there are no objections, please apply.
>
> --
> Alvaro Herrera (<alvherre[a]atentus.com>)
> "La primera ley de las demostraciones en vivo es: no trate de usar el sistema.
> Escriba un gui?n que no toque nada para no causar da?os." (Jakob Nielsen)

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

I have backed out the older patch and replaced it with this new one.

Thanks.

---------------------------------------------------------------------------

Alvaro Herrera wrote:
> Tom Lane dijo:
>
> > Looks like Alvaro got sideswiped by the system catalog indexing changes
> > I made over the weekend. It's a simple change, just reduce the whole
> > mess to a "CatalogUpdateIndexes()" call.
>
> I update two tuples, so I manually CatalogOpenIndexes() and
> CatalogIndexInsert() two times, as per comments in
> CatalogUpdateIndexes().
>
> I also removed a couple of useless CommandCounterIncrement(), some
> useless definitions in src/include/commands/cluster.h and useless
> includes in src/backend/commands/cluster.c. This version passes the
> regression test I had made for previous versions.
>
> If it applies cleanly, please apply :-)
>
> --
> Alvaro Herrera (<alvherre[a]atentus.com>)
> La web junta la gente porque no importa que clase de mutante sexual seas,
> tienes millones de posibles parejas. Pon "buscar gente que tengan sexo con
> ciervos incendi?nse", y el computador dir? "especifique el tipo de ciervo"
> (Jason Alexander)

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

I also added a mention of ANALYZE after CLUSTER to reference page.

---------------------------------------------------------------------------

Alvaro Herrera wrote:
> Tom Lane dijo:
>
> > Looks like Alvaro got sideswiped by the system catalog indexing changes
> > I made over the weekend. It's a simple change, just reduce the whole
> > mess to a "CatalogUpdateIndexes()" call.
>
> I update two tuples, so I manually CatalogOpenIndexes() and
> CatalogIndexInsert() two times, as per comments in
> CatalogUpdateIndexes().
>
> I also removed a couple of useless CommandCounterIncrement(), some
> useless definitions in src/include/commands/cluster.h and useless
> includes in src/backend/commands/cluster.c. This version passes the
> regression test I had made for previous versions.
>
> If it applies cleanly, please apply :-)
>
> --
> Alvaro Herrera (<alvherre[a]atentus.com>)
> La web junta la gente porque no importa que clase de mutante sexual seas,
> tienes millones de posibles parejas. Pon "buscar gente que tengan sexo con
> ciervos incendi?nse", y el computador dir? "especifique el tipo de ciervo"
> (Jason Alexander)

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073