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
