[
https://issues.apache.org/jira/browse/DERBY-2487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12693873#action_12693873
]
Rick Hillegas commented on DERBY-2487:
--------------------------------------
Thanks for the patch, Bryan. I have a couple comments:
XPLAINUtil.getStatementType():
In addition to upper-casing the sql text, I think that you should also trim()
leading whitespace before you look for DML tokens.
Catalogs:
Maybe I'm mis-reading the patch or maybe I grabbed the wrong patch, but it
seems to me that this patch still creates system tables to hold the plan
statistics. I do like the intended approach of creating the tables in a
user-specified schema.
LanguageConnectionContext.setXplainMode() and getXPlainMode():
I think it would be good to introduce some named constants for these modes and
then use them instead of hard-coded numbers.
General approach:
There seem to be three jobs which the statistics collector has to perform:
1) Pick an order in which to walk the graph.
2) Find node-specific details.
3) Decide where and how to persist the details.
This work is divided between two authorities:
A) The swarm of node-specific statistics holders in package
org.apache.derby.impl.sql.execute.rts
B) The visitor, XPLAINSystemTableVisitor
(For the record, I'm puzzled by the swarm of statistics holders in (A). That
suggests to me an old mis-factoring of the problem--but that's an ancient
design decision which falls outside the scope of this improvement.)
(A) is responsible for (1) and (B) is responsible for (3). That division makes
sense to me. This structure will make it possible to write visitors which do
something fancier, like fork the statistics stream to a monitoring process or
feed the forked stream back into the optimizer so that the optimizer can learn
from its mistakes.
However, I'm puzzled that (B) is also responsible for (2). As a consequence of
this decision, we see a lot of overloads of visit() in (B). Anyone who wants to
implement an alternative visitor will have a lot of work to do. I wonder if
this means that (2) should be done by (A) and that we should add a
recordDetails() method to the XPLAINable interface. Something like this:
public interface XPLAINable
{
// determines the order in which to walk the graph
public void accept(...);
// find the details in this node which need to be recorded
public void recordDetails(...);
}
Thanks,
-Rick
> Enhance Derby with EXPLAIN Functionality
> ----------------------------------------
>
> Key: DERBY-2487
> URL: https://issues.apache.org/jira/browse/DERBY-2487
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.2.2.0
> Reporter: Felix Beyer
> Assignee: Bryan Pendleton
> Priority: Minor
> Attachments: Derby physical XPLAIN schema.png,
> incorporateTrunkChanges.diff, removeSourceDepth.diff, RSProtocolNew.pdf,
> rts.xls, small logical xplain schema.pdf, startRegressionTest.diff,
> startRegressionTest.diff, startUpgradeTests.diff, updateRegressionTests.diff,
> updateRegressionTests.diff, usage.txt, userSchemaPrototyping.diff,
> xplain_patch_v1.txt, xplainClasses.pdf
>
>
> This enhancement extends Derby with EXPLAIN functions. Users want to have
> more feedback than they`re getting with the current RuntimeStatistics
> facility. This extension is based on the
> RuntimeStatistics/ResultSetStatistics functions / classes.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
