Re: IDLE in transaction introspection

On 11/15/2011 09:44 AM, Scott Mead wrote:
> Fell off the map last week, here's v2 that:
>  * RUNNING => active
>  * all states from CAPS to lower case
>

This looks like what I was hoping someone would add here now.  Patch 
looks good, only issue I noticed was a spaces instead of a tab goof 
where you set beentry_>st_state at line 2419 in 
src/backend/postmaster/pgstat.c

Missing pieces:

-There is one regression test that uses pg_stat_activity that is broken now.
-The documentation should list the new field and all of the states it 
might include.  That's a serious doc update from the minimal info 
available right now.

I know this issue has been beat up already some, but let me summarize 
and extend that thinking a moment.  I see two equally valid schools of 
thought on how exactly to deal with introducing this change:

-Add the new state field just as you've done it, but keep updating the 
query text anyway.  Do not rename current_query.  Declare the 
overloading of current_query as both a state and the query text to be 
deprecated in 9.3.  This would keep existing tools working fine against 
9.2 and give a clean transition period.

-Forget about backward compatibility and just put all the breaking stuff 
we've been meaning to do in here.  If we're going to rename 
current_query to query--what Scott's patch does here--that will force 
all code using pg_stat_activity to be rewritten.  This seems like the 
perfect time to also change "procpid" to "pid", finally blow away that wart.

I'll happily update all of the tools and samples I deal with to support 
this change.  Most of the ones I can think of will be simplified; 
they're already parsing query_text and extracting the implicit state.  
Just operating on an explicit one instead will be simpler and more robust.

-- 
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us