db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Satheesh Bandaram <sathe...@Sourcery.Org>
Subject Re: [PATCH] Derby 424 - Queryplan for a query using SESSION schema view is incorrectly put in statement cache. This could cause incorrect plan getting executed later if a temp. table is created with that name.
Date Thu, 20 Oct 2005 17:51:38 GMT
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
  <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
  <title></title>
</head>
<body bgcolor="#ffffff" text="#000000">
Thanks for more explanation. Patch looks good and is committed. Should
I port this to 10.1.2 branch?<br>
<br>
Sending&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
java\engine\org\apache\derby\impl\sql\GenericPreparedStatement.java<br>
Sending&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
java\engine\org\apache\derby\impl\sql\GenericStatement.java<br>
Sending&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
java\engine\org\apache\derby\impl\sql\compile\FromList.java<br>
Sending&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
java\testing\org\apache\derbyTesting\functionTests\master\declareGlobalTempTableJava.out<br>
Sending&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
java\testing\org\apache\derbyTesting\functionTests\tests\lang\declareGlobalTempTableJava.java<br>
Transmitting file data .....<br>
Committed revision 326959.<br>
<br>
Mamta Satoor wrote:
<blockquote
 cite="midd9619e4a0510192149l1f960b0h566a44035b96d53f@mail.gmail.com"
 type="cite">
  <p>Hi Satheesh,</p>
  <div>First of all, thanks for reviewing the patch for me. Here are
the answers to your questions 1 and 3.<br>
&nbsp;</div>
  <div>1)If you look at the GenericStatement's prepMinion method,
towards the beginning(line 167 onwards in the review package that I
sent for GenericStatement), we look for the statement in the cache. If
it is found there, we set foundInCache to true. After that, we check if
the statement found in the cache might be referencing SESSION schema
object and if yes, then we do not want to use the statement plan from
the cache, instead we want to build&nbsp;it again. The check to see if
statement references SESSION schema is done by following code in
GenericStatement's prepMinion&nbsp;method
  <br>
&nbsp;&nbsp;&nbsp;&nbsp;if (foundInCache) {<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if (preparedStmt.referencesSessionSchema())
{<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// cannot use this state since
it is private to a connection.<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;// switch to a new statement.<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;foundInCache = false;
  <br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;preparedStmt = new GenericPreparedStatement(this);<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;break;<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;}<br>
&nbsp;&nbsp;&nbsp;&nbsp;}<br>
GenericPreparedStatement or GenericStatement at the time of the check
above donot have the query tree object and hence we can't simply call
qt.referencesSessionSchema. For this reason, I had to add <br>
public boolean referencesSessionSchema(QueryTreeNode qt) to
GenericPreparedStatement(which gets called by the compile phase after
the qt object is constructed)&nbsp;and the method saves the qt object's
referencesSessionSchema() status in it's own local variable
referencesSessionSchema. This local information is what will be used by
the other referencesSessionSchema method to determine if statement
found in cache should be discarded and compile phase should be
re-executed.
  </div>
  <div>&nbsp;</div>
  <div>So, to summarize, the new method after the bind phase, extracts
the referencesSessionSchema information from the passed qt object and
saves it in the local variable.</div>
  <div>&nbsp;public boolean referencesSessionSchema(QueryTreeNode qt)<br>
&nbsp;throws StandardException {<br>
&nbsp;&nbsp;//If the query references a SESSION schema table (temporary or
permanent), then mark so in this statement<br>
&nbsp;&nbsp;referencesSessionSchema = qt.referencesSessionSchema();<br>
&nbsp;&nbsp;return(referencesSessionSchema);<br>
&nbsp;}<br>
&nbsp;</div>
  <div>And, the existing&nbsp;referencesSessionSchema() method as shown
below, uses the local variable (which was filled correctly last time
the statement was compiled) to let the compile phase know if it should
ignore the plan found in the cache and reconstruct the plan because the
old plan had references to SESSION schema objects.
  </div>
  <div>&nbsp;public boolean referencesSessionSchema()<br>
&nbsp;{<br>
&nbsp;&nbsp;return referencesSessionSchema;<br>
&nbsp;}<br>
  <br>
3)At the end of the bind phase for select * from session.st1;
GenericStatement's qt (QueryTreeNode object which in this case is
CursorNode) object has it's resultSet object as a SelectNode which has
a fromList object with referencesSessionSchema field set to true
because it was referencing an object from SESSION schema. </div>
  <div>When the optimize code is called on this bound qt object, the
optimizer replaces the SelectNode resultSet object with a
ProjectRestrictNode and in that process, we loose the
referencesSessionSchema information which was part of the SelectNode's
FromList object. Rather than trying to have that information some how
be transferred to the new ResultSet object,&nbsp;it is more efficient to use
the information right after bind phase and remove the plan from the
statement cache. </div>
  <div>&nbsp;</div>
  <div>Hope this answers your comments.<br>
Mamta<br>
  <br>
&nbsp;</div>
  <div><span class="gmail_quote">On 10/19/05, <b
 class="gmail_sendername">Satheesh Bandaram</b> &lt;<a
 href="mailto:satheesh@sourcery.org">satheesh@sourcery.org</a>&gt;
wrote:</span>
  <blockquote class="gmail_quote"
 style="border-left: 1px solid rgb(204, 204, 204); margin: 0px 0px 0px 0.8ex; padding-left:
1ex;">Thanks
Mamta for the patch. I just have some comments and questions... Thanks
for commenting the changes well!
    <br>
    <br>
1) Why there is a new method in GenericPreparedStatement? This doesn't
return any info about the PreparedStatement itself, so, does this
belong here? Why not just have the check qt.referencesSessionSchema()
in GenericStatement.java
?<br>
    <blockquote>+ public boolean referencesSessionSchema(QueryTreeNode
qt) <br>
+ throws StandardException {<br>
+ //If the query references a SESSION schema table (temporary or
permanent), then mark so in this statement<br>
+ referencesSessionSchema = qt.referencesSessionSchema();<br>
+ return(referencesSessionSchema);<br>
+ } <br>
    </blockquote>
2) Thanks for changing completeCompile() to NOT return
referencesSessionSchema flag... Seems like an ugly way to do it.<br>
    <br>
3)&nbsp; You also mentioned:
    <span class="q"><br>
    <blockquote>This information is again lost during the optimization
and generate phase and hence I moved the check for<br>
SESSION schema reference to right after the bind phase in
GenericStatement.<br>
    </blockquote>
    </span>
    <blockquote>Do you know why this info is lost?<br>
    </blockquote>
Thanks for the good patch.<br>
    <span class="sg"><br>
Satheesh</span>
    <div><span class="e" id="q_1070a523a3f5a8b6_4"><br>
    <br>
Mamta Satoor wrote:
    <blockquote
 cite="http://midd9619e4a0510182215v1c89581drdd0b3596a184ab42@mail.gmail.com"
 type="cite">
      <div>Hi,</div>
      <div>&nbsp;</div>
      <div>I have attached a review package for this bug to JIRA,
hopefully, in time for 10.1.2 release. </div>
      <p>The files affected by this change are<br>
M&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; java\engine\org\apache\derby\impl\sql\GenericStatement.java<br>
M&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; java\engine\org\apache\derby\impl\sql\compile\FromList.java<br>
M&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
java\engine\org\apache\derby\impl\sql\GenericPreparedStatement.java <br>
M&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
java\testing\org\apache\derbyTesting\functionTests\tests\lang\declareGlobalTempTableJava.java<br>
M&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
java\testing\org\apache\derbyTesting\functionTests\master\declareGlobalTempTableJava.out</p>
      <p>The changes for this fix are very localized, affecting only 3
files in Derby engine. Basically, the problem is that, during the
compile phase of views, the reference to the view gets replaced by the
view definition, which causes us to loose the information that the view
might have belonged in SESSION schema. In order to fix this, during the
bind phase in FromList, before the view gets replaced by its
definition, I find out if the view is from SESSION schema, If yes, then
I save this information in FromList and this gets used by FromList when
it is asked if it has any items referencing SESSION schema objects.
This information is again lost during the optimization and generate
phase and hence I moved the check for SESSION schema reference to right
after the bind phase in GenericStatement. If there is a reference to
SESSION schema object, GenericStatement will remove the statement from
the cache. </p>
      <div>I have put in quite a big of comments in the code which
hopefully will make the patch easier to understand. I have added a new
test for this and have run all the tests with no failures using Sun's
jdk142 on Windows XP machine. </div>
      <div>&nbsp;</div>
      <div>thanks,</div>
      <div>Mamta</div>
    </blockquote>
    </span></div>
  </blockquote>
  </div>
  <br>
</blockquote>
</body>
</html>


Mime
View raw message