Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-18 Thread Alvaro Herrera
Michael Paquier wrote: I had a look at your modified version, and it looks good to me. Thanks, pushed. (Without the va_cols change proposed downthread.) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services --

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-18 Thread Michael Paquier
On Thu, Mar 19, 2015 at 2:44 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Michael Paquier wrote: I had a look at your modified version, and it looks good to me. Thanks, pushed. (Without the va_cols change proposed downthread.) Thanks a lot! I will shortly work on the rebase for the

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-17 Thread Magnus Hagander
On Tue, Mar 17, 2015 at 6:31 PM, Stephen Frost sfr...@snowman.net wrote: * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: I went to change this patch status in the commitfest app, and the app told me I cannot change the status in the current commitfest. Please somebody with commitfest

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-17 Thread Stephen Frost
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: I went to change this patch status in the commitfest app, and the app told me I cannot change the status in the current commitfest. Please somebody with commitfest mace superpowers set it as ready for committer. I'm afraid the issue is a

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-17 Thread Alvaro Herrera
Michael Paquier wrote: On Wed, Mar 18, 2015 at 2:22 AM, Alvaro Herrera wrote: 1. ordered the argument list to vacuum(), hopefully it's more sensible now. Fine for me. Actually, why don't we move va_cols to VacuumParams too? -- Álvaro Herrerahttp://www.2ndQuadrant.com/

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-17 Thread Michael Paquier
On Wed, Mar 18, 2015 at 10:51 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Michael Paquier wrote: On Wed, Mar 18, 2015 at 2:22 AM, Alvaro Herrera wrote: 1. ordered the argument list to vacuum(), hopefully it's more sensible now. Fine for me. Actually, why don't we move va_cols to

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-17 Thread Michael Paquier
On Wed, Mar 18, 2015 at 2:22 AM, Alvaro Herrera wrote: Here's an updated patch. I took your latest version and made some extra changes: Thanks for taking the time to look at it! 1. ordered the argument list to vacuum(), hopefully it's more sensible now. Fine for me. 2. changed struct

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-17 Thread Alvaro Herrera
I went to change this patch status in the commitfest app, and the app told me I cannot change the status in the current commitfest. Please somebody with commitfest mace superpowers set it as ready for committer. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-11 Thread Jim Nasby
On 3/11/15 3:57 PM, Tom Lane wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: But autovacuum is still manufacturing a VacuumStmt by hand. If we want to get rid of that, I think it'd work to have a new ExecVacuum(VacuumStmt, params) function which is called from standard_ProcessUtility

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-11 Thread Tom Lane
Alvaro Herrera alvhe...@2ndquadrant.com writes: But autovacuum is still manufacturing a VacuumStmt by hand. If we want to get rid of that, I think it'd work to have a new ExecVacuum(VacuumStmt, params) function which is called from standard_ProcessUtility and does just vacuum(rel, relid,

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-11 Thread Robert Haas
On Fri, Mar 6, 2015 at 1:39 AM, Michael Paquier michael.paqu...@gmail.com wrote: Do you mean removing totally VacuumStmt from the stack? We would then need to add relation and va_cols as additional arguments of things like vacuum_rel, analyze_rel, do_analyze_rel or similar. FWIW, adding

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-11 Thread Alvaro Herrera
Robert Haas wrote: On Fri, Mar 6, 2015 at 1:39 AM, Michael Paquier michael.paqu...@gmail.com wrote: - 0001 is the previous one - 0002 removes VacuumStmt from the call stack of ANALYZE and VACUUM routines - 0003 moves for_wraparound in VacuumParams. Yeah, I think something like this

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-11 Thread Robert Haas
On Wed, Mar 11, 2015 at 3:09 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas wrote: On Fri, Mar 6, 2015 at 1:39 AM, Michael Paquier michael.paqu...@gmail.com wrote: - 0001 is the previous one - 0002 removes VacuumStmt from the call stack of ANALYZE and VACUUM routines -

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-05 Thread Robert Haas
On Thu, Mar 5, 2015 at 9:58 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas wrote: On Sun, Mar 1, 2015 at 6:58 AM, Michael Paquier michael.paqu...@gmail.com wrote: Hm, why not. That would remove all inconsistencies between the parser and the autovacuum code path. Perhaps

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-05 Thread Stephen Frost
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Robert Haas wrote: On Sun, Mar 1, 2015 at 6:58 AM, Michael Paquier michael.paqu...@gmail.com wrote: Hm, why not. That would remove all inconsistencies between the parser and the autovacuum code path. Perhaps something like the

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-05 Thread Alvaro Herrera
Robert Haas wrote: On Sun, Mar 1, 2015 at 6:58 AM, Michael Paquier michael.paqu...@gmail.com wrote: Hm, why not. That would remove all inconsistencies between the parser and the autovacuum code path. Perhaps something like the attached makes sense then? I really don't see this patch, or

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-05 Thread Michael Paquier
On Fri, Mar 6, 2015 at 12:42 AM, Robert Haas wrote: On Thu, Mar 5, 2015 at 9:58 AM, Alvaro Herrera wrote: Here's a simple idea to improve the patch: make VacuumParams include VacuumStmt and the for_wraparound and do_toast flags. ISTM that reduces the number of arguments to be passed down in a

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-04 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote: On Sun, Mar 1, 2015 at 6:58 AM, Michael Paquier michael.paqu...@gmail.com wrote: Hm, why not. That would remove all inconsistencies between the parser and the autovacuum code path. Perhaps something like the attached makes sense then? I

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-04 Thread Robert Haas
On Sun, Mar 1, 2015 at 6:58 AM, Michael Paquier michael.paqu...@gmail.com wrote: Hm, why not. That would remove all inconsistencies between the parser and the autovacuum code path. Perhaps something like the attached makes sense then? I really don't see this patch, or any of the previous ones,

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-03-04 Thread Michael Paquier
On Thu, Mar 5, 2015 at 12:54 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Sun, Mar 1, 2015 at 6:58 AM, Michael Paquier michael.paqu...@gmail.com wrote: Hm, why not. That would remove all inconsistencies between the parser and the autovacuum

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-28 Thread Stephen Frost
Michael, * Michael Paquier (michael.paqu...@gmail.com) wrote: On Sat, Feb 28, 2015 at 2:45 PM, Stephen Frost sfr...@snowman.net wrote: So, basically, this feels like it's not really the right place for these checks and if there is an existing problem then it's probably with the grammar...

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-28 Thread Michael Paquier
On Sat, Feb 28, 2015 at 2:45 PM, Stephen Frost sfr...@snowman.net wrote: I'm trying to wrap my head around the reasoning for this also and not sure I'm following. In general, I don't think we protect all that hard against functions being called with tokens that aren't allowed by the parse.

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-27 Thread Stephen Frost
Michael, * Michael Paquier (michael.paqu...@gmail.com) wrote: On Wed, Feb 18, 2015 at 12:09 AM, Robert Haas robertmh...@gmail.com wrote: I think it's right the way it is. The parser constructs a VacuumStmt for either a VACUUM or an ANALYZE command. That statement is checking that if you

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-19 Thread Peter Eisentraut
On 2/18/15 1:26 AM, Michael Paquier wrote: On Wed, Feb 18, 2015 at 10:06 AM, Michael Paquier wrote: Yes, the existing assertion is right. My point is that it is strange that we do not check the values of freeze parameters for an ANALYZE query, which should be set to -1 all the time. If this is

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-19 Thread Michael Paquier
On Fri, Feb 20, 2015 at 5:41 AM, Peter Eisentraut wrote: That's cool if you want to add those assertions, but please make them separate statements each, like Assert(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE) || vacstmt-freeze_min_age == -1); Assert(vacstmt-options (VACOPT_FULL |

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-17 Thread Michael Paquier
On Wed, Feb 18, 2015 at 12:09 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, Feb 12, 2015 at 11:54 PM, Michael Paquier michael.paqu...@gmail.com wrote: Hi all, When calling vacuum(), there is the following assertion using VACOPT_FREEZE: Assert((vacstmt-options VACOPT_VACUUM) ||

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-17 Thread Michael Paquier
On Wed, Feb 18, 2015 at 10:06 AM, Michael Paquier wrote: Yes, the existing assertion is right. My point is that it is strange that we do not check the values of freeze parameters for an ANALYZE query, which should be set to -1 all the time. If this is thought as not worth checking, I'll drop

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-17 Thread Robert Haas
On Thu, Feb 12, 2015 at 11:54 PM, Michael Paquier michael.paqu...@gmail.com wrote: Hi all, When calling vacuum(), there is the following assertion using VACOPT_FREEZE: Assert((vacstmt-options VACOPT_VACUUM) || !(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE))); I think that this should

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-13 Thread Jim Nasby
On 2/12/15 10:54 PM, Michael Paquier wrote: Hi all, When calling vacuum(), there is the following assertion using VACOPT_FREEZE: Assert((vacstmt-options VACOPT_VACUUM) || !(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE))); I think that this should be changed with sanity checks based on

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-13 Thread Michael Paquier
On Sat, Feb 14, 2015 at 8:10 AM, Jim Nasby wrote: On 2/12/15 10:54 PM, Michael Paquier wrote: Hi all, When calling vacuum(), there is the following assertion using VACOPT_FREEZE: Assert((vacstmt-options VACOPT_VACUUM) || !(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE))); I think

[HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-12 Thread Michael Paquier
Hi all, When calling vacuum(), there is the following assertion using VACOPT_FREEZE: Assert((vacstmt-options VACOPT_VACUUM) || !(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE))); I think that this should be changed with sanity checks based on the parameter values of freeze_* in VacuumStmt as