Le 23/01/2010 16:40, Magnus Hagander a écrit :
> 2010/1/23 Guillaume Lelarge <[email protected]>:
>> Le 23/01/2010 15:14, Magnus Hagander a écrit :
>>> 2010/1/23 Guillaume Lelarge <[email protected]>:
>>>> Hi,
>>>>
>>>> This patch handles two new parameters for EXPLAIN: COSTS and BUFFERS.
>>>> I'm not sure that we need to add support for format. I would like to
>>>> have your opinion on this?
>>>
>>> Not really. The whole idea of using EXPLAIN on the menu is to get the
>>> graphical output, so I don't see where the user would care aobut the
>>> format of the text behind it.
>>>
>>
>> That was also what I thought... but, if so, why do we propose VERBOSE
>> option? in this case, we don't get a graphical EXPLAIN, but a textual
>> EXPLAIN VERBOSE.
>
> Shows how often I use it. I thought we put that stuff in the main
> explain output, not flick to text mode.
>
> Couldn't we put the verbose stuff in the text output and still show
> the graphical one?
>
Done with Euler's patch.
> Hmm. That might at least be easier to do if we use a machine format explain?
>
>>> That said, we might want to consider switching to using one of the
>>> machine readable formats for newer versions, and then just implement
>>> parsing of these options there. In the end, that should lead to less
>>> complicated code.. (mind you, I didn't even look at your code so I
>>> don't know how complex it is :D)
>>>
>>
>> We can't switch completely if we want to still have compatibility with
>> the older releases. Anyways, this is something we'll have to do to ease
>> the process.
>
> Right. What I'd envision us doing is a simple
> if (version < 9.0)
> run_old_explain_code();
> else
> run_new_explain_code();
>
> and only the codepath for run_new_explain_code() - which would use a
> machine format output - would have the implementation of any of these
> new parameters.
>
> It would be slightly more work to get started since the new parser
> needs to be done (hopefully there are classes that'll do 95% of that
> for us already though), but not having to maintain manual parsing of
> these new options should be a win in the long run.
>
> And eventually, some 5 years from now or so, we can retire the old
> explain code because we no longer support pre-9.0 versions :D
>
OK. I'll add a ticket so that we can take care of this in another patch.
>>> While you're whacking around the explain code, one thing that has
>>> always nagged me, is that ANALYZE is an option for EXPLAIN. IMO we
>>> should have "Explain" and "Explain Analyze" as separate commands,
>>> because they actually do different things. And then options for things
>>> like verbose, costs, buffers etc. If we're going to change that ever,
>>> now is probably the best time :-)
>>>
>>> Thoughts?
>>>
>>
>> I don't quite get why we should do that. You can already have an EXPLAIN
>> or an EXPLAIN ANALYZE. Or is it simply to really differentiate them?
>> because one won't launch the query and the other one will?
>
> Now we have:
> explain
> Explain Options -> [verbose, analyze, newstuff]
>
> I'd prefer
> Explain
> Explain Analyze
> Explain Options -> [verbose, newstuff]
>
Done. F7 launches an explain, Shift-F7 launches an explain analyze.
New patch attached.
--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com
diff --git a/pgadmin/ctl/explainShape.cpp b/pgadmin/ctl/explainShape.cpp
index 01bd216..f58197e 100644
--- a/pgadmin/ctl/explainShape.cpp
+++ b/pgadmin/ctl/explainShape.cpp
@@ -163,6 +163,7 @@ ExplainShape *ExplainShape::Create(long level, ExplainShape *last, const wxStrin
ExplainShape *s=0;
int costPos=str.Find(wxT("(cost="));
+ int actPos = str.Find(wxT("(actual"));
wxStringTokenizer tokens(str, wxT(" "));
wxString token = tokens.GetNextToken();
@@ -171,7 +172,13 @@ ExplainShape *ExplainShape::Create(long level, ExplainShape *last, const wxStrin
wxString token4;
if (tokens.HasMoreTokens())
token4 = tokens.GetNextToken();
- wxString descr = costPos > 0 ? str.Left(costPos) : str;
+ wxString descr;
+ if (costPos > 0)
+ descr = str.Left(costPos);
+ else if (actPos > 0)
+ descr = str.Left(actPos);
+ else
+ descr = str;
// possible keywords can be found in postgresql/src/backend/commands/explain.c
@@ -344,7 +351,6 @@ ExplainShape *ExplainShape::Create(long level, ExplainShape *last, const wxStrin
if (costPos > 0)
{
- int actPos = str.Find(wxT("(actual"));
if (actPos > 0)
{
s->actual = str.Mid(actPos);
@@ -353,6 +359,8 @@ ExplainShape *ExplainShape::Create(long level, ExplainShape *last, const wxStrin
else
s->cost = str.Mid(costPos);
}
+ else if (actPos > 0)
+ s->actual = str.Mid(actPos);
int w=50, h=20;
diff --git a/pgadmin/frm/frmQuery.cpp b/pgadmin/frm/frmQuery.cpp
index b11324a..d03fcd6 100644
--- a/pgadmin/frm/frmQuery.cpp
+++ b/pgadmin/frm/frmQuery.cpp
@@ -108,6 +108,8 @@ EVT_MENU(MNU_EXECUTE, frmQuery::OnExecute)
EVT_MENU(MNU_EXECPGS, frmQuery::OnExecScript)
EVT_MENU(MNU_EXECFILE, frmQuery::OnExecFile)
EVT_MENU(MNU_EXPLAIN, frmQuery::OnExplain)
+EVT_MENU(MNU_EXPLAINANALYZE, frmQuery::OnExplain)
+EVT_MENU(MNU_BUFFERS, frmQuery::OnBuffers)
EVT_MENU(MNU_CANCEL, frmQuery::OnCancel)
EVT_MENU(MNU_AUTOROLLBACK, frmQuery::OnAutoRollback)
EVT_MENU(MNU_CONTENTS, frmQuery::OnContents)
@@ -274,11 +276,13 @@ pgsTimer(new pgScriptTimer(this))
queryMenu->Append(MNU_EXECPGS, _("Execute &pgScript\tF6"), _("Execute pgScript"));
queryMenu->Append(MNU_EXECFILE, _("Execute to file"), _("Execute query, write result to file"));
queryMenu->Append(MNU_EXPLAIN, _("E&xplain\tF7"), _("Explain query"));
+ queryMenu->Append(MNU_EXPLAINANALYZE, _("Explain analyze\tShift-F7"), _("Explain and analyze query"));
wxMenu *eo=new wxMenu();
eo->Append(MNU_VERBOSE, _("Verbose"), _("Explain verbose query"), wxITEM_CHECK);
- eo->Append(MNU_ANALYZE, _("Analyze"), _("Explain analyze query"), wxITEM_CHECK);
+ eo->Append(MNU_COSTS, _("Costs"), _("Explain analyze query with (or without costs)"), wxITEM_CHECK);
+ eo->Append(MNU_BUFFERS, _("Buffers"), _("Explain analyze query with (or without buffers)"), wxITEM_CHECK);
queryMenu->Append(MNU_EXPLAINOPTIONS, _("Explain &options"), eo, _("Options modifying Explain output"));
queryMenu->AppendSeparator();
queryMenu->Append(MNU_SAVEHISTORY, _("Save history"), _("Save history of executed commands."));
@@ -329,7 +333,8 @@ pgsTimer(new pgScriptTimer(this))
SetMenuBar(menuBar);
queryMenu->Check(MNU_VERBOSE, settings->GetExplainVerbose());
- queryMenu->Check(MNU_ANALYZE, settings->GetExplainAnalyze());
+ queryMenu->Check(MNU_COSTS, settings->GetExplainCosts());
+ queryMenu->Check(MNU_BUFFERS, settings->GetExplainBuffers());
UpdateRecentFiles();
@@ -1478,8 +1483,9 @@ void frmQuery::OnClose(wxCloseEvent& event)
controller->nullView(); //to avoid bug on *nix when deleting controller
- settings->SetExplainAnalyze(queryMenu->IsChecked(MNU_ANALYZE));
settings->SetExplainVerbose(queryMenu->IsChecked(MNU_VERBOSE));
+ settings->SetExplainCosts(queryMenu->IsChecked(MNU_COSTS));
+ settings->SetExplainBuffers(queryMenu->IsChecked(MNU_BUFFERS));
sqlResult->Abort(); // to make sure conn is unused
@@ -1828,7 +1834,8 @@ void frmQuery::OnExplain(wxCommandEvent& event)
return;
wxString sql;
int resultToRetrieve=1;
- bool verbose=queryMenu->IsChecked(MNU_VERBOSE), analyze=queryMenu->IsChecked(MNU_ANALYZE);
+ bool verbose=queryMenu->IsChecked(MNU_VERBOSE);
+ bool analyze = event.GetId() == MNU_EXPLAINANALYZE;
if (analyze)
{
@@ -1836,10 +1843,37 @@ void frmQuery::OnExplain(wxCommandEvent& event)
resultToRetrieve++;
}
sql += wxT("EXPLAIN ");
- if (analyze)
- sql += wxT("ANALYZE ");
- if (verbose)
- sql += wxT("VERBOSE ");
+ if (conn->BackendMinimumVersion(8, 5))
+ {
+ bool costs=queryMenu->IsChecked(MNU_COSTS);
+ bool buffers=queryMenu->IsChecked(MNU_BUFFERS);
+
+ sql += wxT("(");
+ if (analyze)
+ sql += wxT("ANALYZE on, ");
+ else
+ sql += wxT("ANALYZE off, ");
+ if (verbose)
+ sql += wxT("VERBOSE on, ");
+ else
+ sql += wxT("VERBOSE off, ");
+ if (costs)
+ sql += wxT("COSTS on, ");
+ else
+ sql += wxT("COSTS off, ");
+ if (buffers)
+ sql += wxT("BUFFERS on ");
+ else
+ sql += wxT("BUFFERS off ");
+ sql += wxT(")");
+ }
+ else
+ {
+ if (analyze)
+ sql += wxT("ANALYZE ");
+ if (verbose)
+ sql += wxT("VERBOSE ");
+ }
int offset=sql.Length();
@@ -1855,6 +1889,11 @@ void frmQuery::OnExplain(wxCommandEvent& event)
execQuery(sql, resultToRetrieve, true, offset, false, true, verbose);
}
+void frmQuery::OnBuffers(wxCommandEvent& event)
+{
+ queryMenu->Enable(MNU_EXPLAIN, !queryMenu->IsChecked(MNU_BUFFERS));
+}
+
// Update the main SQL query from the GQB if desired
bool frmQuery::updateFromGqb(bool executing)
{
@@ -2431,7 +2470,7 @@ void frmQuery::completeQuery(bool done, bool explain, bool verbose)
// If this was an EXPLAIN query, process the results
if (done && explain)
{
- if (!verbose)
+ if (!verbose || conn->BackendMinimumVersion(8, 4))
{
int i;
wxString str;
diff --git a/pgadmin/include/frm/frmQuery.h b/pgadmin/include/frm/frmQuery.h
index 9571197..0b771db 100644
--- a/pgadmin/include/frm/frmQuery.h
+++ b/pgadmin/include/frm/frmQuery.h
@@ -129,6 +129,7 @@ private:
void OnExecScript(wxCommandEvent& event);
void OnExecFile(wxCommandEvent& event);
void OnExplain(wxCommandEvent& event);
+ void OnBuffers(wxCommandEvent& event);
void OnNew(wxCommandEvent& event);
void OnOpen(wxCommandEvent& event);
void OnSave(wxCommandEvent& event);
diff --git a/pgadmin/include/frm/menu.h b/pgadmin/include/frm/menu.h
index febe74a..bb11c7d 100644
--- a/pgadmin/include/frm/menu.h
+++ b/pgadmin/include/frm/menu.h
@@ -62,9 +62,11 @@ enum
MNU_EXECUTE,
MNU_EXECFILE,
MNU_EXPLAIN,
+ MNU_EXPLAINANALYZE,
MNU_EXPLAINOPTIONS,
MNU_VERBOSE,
- MNU_ANALYZE,
+ MNU_COSTS,
+ MNU_BUFFERS,
MNU_AUTOROLLBACK,
MNU_CLEARHISTORY,
MNU_SAVEHISTORY,
diff --git a/pgadmin/include/utils/sysSettings.h b/pgadmin/include/utils/sysSettings.h
index 984f287..95e7682 100644
--- a/pgadmin/include/utils/sysSettings.h
+++ b/pgadmin/include/utils/sysSettings.h
@@ -92,8 +92,10 @@ public:
// Explain options
bool GetExplainVerbose() const { bool b; Read(wxT("frmQuery/ExplainVerbose"), &b, false); return b; }
void SetExplainVerbose(const bool newval) { Write(wxT("frmQuery/ExplainVerbose"), newval); }
- bool GetExplainAnalyze() const { bool b; Read(wxT("frmQuery/ExplainAnalyze"), &b, false); return b; }
- void SetExplainAnalyze(const bool newval) { Write(wxT("frmQuery/ExplainAnalyze"), newval); }
+ bool GetExplainCosts() const { bool b; Read(wxT("frmQuery/ExplainCosts"), &b, true); return b; }
+ void SetExplainCosts(const bool newval) { Write(wxT("frmQuery/ExplainCosts"), newval); }
+ bool GetExplainBuffers() const { bool b; Read(wxT("frmQuery/ExplainBuffers"), &b, false); return b; }
+ void SetExplainBuffers(const bool newval) { Write(wxT("frmQuery/ExplainBuffers"), newval); }
// Display options
wxString GetSystemSchemas() const { wxString s; Read(wxT("SystemSchemas"), &s, wxEmptyString); return s; }
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
