Re: pgsql: REINDEX SCHEMA

REINDEX SCHEMA

Add new SCHEMA option to REINDEX and reindexdb.

Sawada Masahiko

Reviewed by Michael Paquier and Fabrízio de Royes Mello

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/fe263d115a7dd16095b8b8f1e943aff2bb4574d2

Modified Files
--------------
doc/src/sgml/ref/reindex.sgml | 15 +++-
doc/src/sgml/ref/reindexdb.sgml | 22 ++++++
src/backend/commands/indexcmds.c | 110 ++++++++++++++++++++--------
src/backend/parser/gram.y | 35 +++++----
src/backend/tcop/utility.c | 15 ++--
src/bin/psql/tab-complete.c | 4 +-
src/bin/scripts/reindexdb.c | 38 +++++++++-
src/bin/scripts/t/090_reindexdb.pl | 7 +-
src/include/commands/defrem.h | 3 +-
src/include/nodes/parsenodes.h | 11 ++-
src/test/regress/expected/create_index.out | 31 ++++++++
src/test/regress/sql/create_index.sql | 23 ++++++
12 files changed, 253 insertions(+), 61 deletions(-)

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> REINDEX SCHEMA

Buildfarm member crake doesn't seem to like your change in
src/bin/scripts/t/090_reindexdb.pl. I'm not sure why not.

regards, tom lane

On 9 December 2014 at 01:40, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> REINDEX SCHEMA
>
> Buildfarm member crake doesn't seem to like your change in
> src/bin/scripts/t/090_reindexdb.pl. I'm not sure why not.

Yeh, it looks like some commands need 2 tests, others. So this was a
breakage of the test script itself, not the actual patch.

It seems we're not running bin-check in many places.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 9 December 2014 at 01:56, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 9 December 2014 at 01:40, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>>> REINDEX SCHEMA
>>
>> Buildfarm member crake doesn't seem to like your change in
>> src/bin/scripts/t/090_reindexdb.pl. I'm not sure why not.
>
> Yeh, it looks like some commands need 2 tests, others. So this was a
> breakage of the test script itself, not the actual patch.

"some commands need 2 tests, others 3"

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> REINDEX SCHEMA

The results from jagarundi and leech suggest that more attention needs to
be paid to ensuring that tables are reindexed in a consistent order.
Either that, or you're going to have to dumb down the regression test.

regards, tom lane

On Tue, Dec 9, 2014 at 3:31 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> REINDEX SCHEMA
>
> The results from jagarundi and leech suggest that more attention needs to
> be paid to ensuring that tables are reindexed in a consistent order.
> Either that, or you're going to have to dumb down the regression test.

Hm. The diff is clear:
***************
*** 2852,2859 ****
SET SESSION ROLE user_reindex;
ERROR: role "user_reindex" does not exist
REINDEX SCHEMA schema_to_reindex;
- NOTICE: table "schema_to_reindex.table1" was reindexed
NOTICE: table "schema_to_reindex.table2" was reindexed
-- Clean up
RESET ROLE;
DROP ROLE user_reindex;
--- 2852,2859 ----
SET SESSION ROLE user_reindex;
ERROR: role "user_reindex" does not exist
REINDEX SCHEMA schema_to_reindex;
NOTICE: table "schema_to_reindex.table2" was reindexed
+ NOTICE: table "schema_to_reindex.table1" was reindexed
-- Clean up
RESET ROLE;
DROP ROLE user_reindex;

We could store the results in an array instead of a list and apply a
qsort to it, but that would be costly if there are many relations
involved in the reindex. Hence I guess raising client_min_messages to
warning is fine? I'll send a patch in the REINDEX SCHEMA thread,
groupped with a couple of other fixes to problems I just found.
--
Michael

On Tue, Dec 9, 2014 at 4:56 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Tue, Dec 9, 2014 at 3:31 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>>> REINDEX SCHEMA
>>
>> The results from jagarundi and leech suggest that more attention needs to
>> be paid to ensuring that tables are reindexed in a consistent order.
>> Either that, or you're going to have to dumb down the regression test.
>
> Hm. The diff is clear:
> ***************
> *** 2852,2859 ****
> SET SESSION ROLE user_reindex;
> ERROR: role "user_reindex" does not exist
> REINDEX SCHEMA schema_to_reindex;
> - NOTICE: table "schema_to_reindex.table1" was reindexed
> NOTICE: table "schema_to_reindex.table2" was reindexed
> -- Clean up
> RESET ROLE;
> DROP ROLE user_reindex;
> --- 2852,2859 ----
> SET SESSION ROLE user_reindex;
> ERROR: role "user_reindex" does not exist
> REINDEX SCHEMA schema_to_reindex;
> NOTICE: table "schema_to_reindex.table2" was reindexed
> + NOTICE: table "schema_to_reindex.table1" was reindexed
> -- Clean up
> RESET ROLE;
> DROP ROLE user_reindex;
>
> We could store the results in an array instead of a list and apply a
> qsort to it, but that would be costly if there are many relations
> involved in the reindex. Hence I guess raising client_min_messages to
> warning is fine? I'll send a patch in the REINDEX SCHEMA thread,
> groupped with a couple of other fixes to problems I just found.
Bonus thought: using a plpgsql function that saves relfilenode before
REINDEX SCHEMA and checks that they are updated to different values
after the operation..
--
Michael

On 9 December 2014 at 15:31, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> REINDEX SCHEMA
>
> The results from jagarundi and leech suggest that more attention needs to
> be paid to ensuring that tables are reindexed in a consistent order.
> Either that, or you're going to have to dumb down the regression test.

OK, thanks, looking again now.

Do you get emails when something fails?

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Simon Riggs wrote:

> Do you get emails when something fails?

The buildfarm system has a number of mailing lists for automated notices
when stuff fails; see http://buildfarm.postgresql.org/ and click on
"email lists and status archives". They run on *gasp* pgfoundry!

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> Do you get emails when something fails?

No. I used to, but buildfarm-status-green has been dysfunctional for
years now. These days I just look at the status page every so often.

regards, tom lane

On 9 December 2014 at 23:17, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> Do you get emails when something fails?
>
> No. I used to, but buildfarm-status-green has been dysfunctional for
> years now. These days I just look at the status page every so often.

OK, thanks for checking. I won't sign up then.

Ironic that I was expecting problems from the Windows patch, not this one.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 12/09/2014 09:17 AM, Tom Lane wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> Do you get emails when something fails?
> No. I used to, but buildfarm-status-green has been dysfunctional for
> years now. These days I just look at the status page every so often.
>
>

That's the first I recall having heard of this. I am still getting a
digest every day or two for that list, so I'm curious to know what's not
working.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 12/09/2014 09:17 AM, Tom Lane wrote:
>> No. I used to, but buildfarm-status-green has been dysfunctional for
>> years now. These days I just look at the status page every so often.

> That's the first I recall having heard of this. I am still getting a
> digest every day or two for that list, so I'm curious to know what's not
> working.

Really? I'm subscribed to buildfarm-status-green-digest, but to call its
delivery erratic would be an understatement.

Digging in my mail logs, my recollection that it'd been broken for more
than a year seems to be wrong: it looks like I got roughly-daily email
up to about Feb 20 2014. Since then, though, this has been the sum total
of email from that source:

From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Thu Feb 20 07:01:21 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Sat Mar 29 02:52:04 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Mon Apr 7 15:49:05 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Mon May 12 18:05:23 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Wed May 28 20:19:38 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Fri Jun 13 20:56:49 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Tue Jul 15 13:41:20 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Thu Aug 7 05:01:56 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Fri Aug 29 07:04:04 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Wed Oct 1 13:55:23 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Sat Oct 25 15:30:02 2014
From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Mon Dec 1 12:21:45 2014

Every so often it will burp out a digest that seems to cover a month or
more worth of events, which of course is both bulky and uselessly late.

regards, tom lane

Tom Lane wrote:

> Digging in my mail logs, my recollection that it'd been broken for more
> than a year seems to be wrong: it looks like I got roughly-daily email
> up to about Feb 20 2014. Since then, though, this has been the sum total
> of email from that source:
>
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Thu Feb 20 07:01:21 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Sat Mar 29 02:52:04 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Mon Apr 7 15:49:05 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Mon May 12 18:05:23 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Wed May 28 20:19:38 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Fri Jun 13 20:56:49 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Tue Jul 15 13:41:20 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Thu Aug 7 05:01:56 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Fri Aug 29 07:04:04 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Wed Oct 1 13:55:23 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Sat Oct 25 15:30:02 2014
> From pgbuildfarm-status-green-bounces(at)lists(dot)pgfoundry(dot)org Mon Dec 1 12:21:45 2014
>
> Every so often it will burp out a digest that seems to cover a month or
> more worth of events, which of course is both bulky and uselessly late.

pgfoundry.org itself is not working terribly well apparently. Why can't
we just move these lists to the host that runs the buildfarm web server?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Dec 9, 2014 at 12:31 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> REINDEX SCHEMA
>
> Add new SCHEMA option to REINDEX and reindexdb.
>
> Sawada Masahiko
>
> Reviewed by Michael Paquier and Fabrízio de Royes Mello
>

I noticed that the unused variable in ReindexStmt has not been removed yet.
We changed to logic of ReindexObject to avoid boolean dance.
Attached patch rid of them(do_user and do_system variable).

Regards,

-------
Sawada Masahiko

On 12/10/2014 12:02 AM, Tom Lane wrote:
>
> Every so often it will burp out a digest that seems to cover a month or
> more worth of events, which of course is both bulky and uselessly late.

I have made a change which might fix this. Let's see how it goes. I'll
try to keep an eye on things until I get it working.

cheers

andrew

On Wed, Dec 10, 2014 at 9:53 PM, Sawada Masahiko <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Tue, Dec 9, 2014 at 12:31 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> REINDEX SCHEMA
>>
>> Add new SCHEMA option to REINDEX and reindexdb.
>>
>> Sawada Masahiko
>>
>> Reviewed by Michael Paquier and Fabrízio de Royes Mello
>>
>
> I noticed that the unused variable in ReindexStmt has not been removed yet.
> We changed to logic of ReindexObject to avoid boolean dance.
> Attached patch rid of them(do_user and do_system variable).

Applied. Thanks!

Regards,

--
Fujii Masao