db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "A B (JIRA)" <j...@apache.org>
Subject [jira] Updated: (DERBY-3494) Move the setup of NormalizeResultSetNode into the NormalizeResultSetNode
Date Fri, 07 Mar 2008 23:07:46 GMT

     [ https://issues.apache.org/jira/browse/DERBY-3494?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

A B updated DERBY-3494:
-----------------------

    Attachment: d3494_npe_writeup.html

> Since the ResultColumnList of the original ResultSetNode correctly describes
> the desired outcome, it's not clear to me why NormalizeResultSetNode can't
> just refer to the same list and use it for its processing.

...

> I am trying to understand why we get the npe in views.sql with the small
> code change proposed [ in DERBY-3310 ].

For reference, the change we're talking about here is in ResultSetNode.java:

@@ -1430,8 +1430,7 @@

         /* We get a shallow copy of the ResultColumnList and its
          * ResultColumns.  (Copy maintains ResultColumn.expression for now.)
          */
  -        ResultColumnList prRCList = resultColumns;
  -        resultColumns = resultColumns.copyListAndObjects();
  +        ResultColumnList prRCList = resultColumns.copyListAndObjects();

         // Remove any columns that were generated.
         prRCList.removeGeneratedGroupingColumns();

In reading the javadoc comments for genNormalizeResultSetNode(...), which is the method in
which the above fragment appears, it definitely seems like the original lines of code were
deliberate: i.e. that the decision to set prRCList to resultColumns and then make resultColumns
point to a copy of itself was intentional.  I could not, however, determine from the comments
*why* exactly this was necessary.  So I did some tracing through code generation with and
without the change to try to see what is going on.

The short explanation comes down to this: By doing what the code currently does we ensure
that any column references which point to the result column list of the node in question (UnionNode
in the case of npe.sql)--and in particular, column references within predicates that sit higher
up in the query tree--will always point to the "top" of the query subtree that is generated
for the node. That in turn ensures that, during execution, the references will point to rows
which come from the correct result set.  The current code in genNormalizeResultSetNode (and
elsewhere) accomplishes this by _moving_ the target node's ResultColumnList object, which
may or may not be referenced elsewhere, up to the "top" of the generated subtree.

That said, the small change shown above ultimately makes it so that predicates which point
to the UnionNode's result column list will be applied directly against rows from the *UnionNode*
instead of against rows from the node generated on *top* of the UnionNode.  That causes the
predicate to attempt to reference values from the wrong execution-time result set, which in
turn leads to a NullPointerException.

If that's good enough to satsify your curiosity, then you can stop reading now :)  Otherwise,
a more detailed (and hopefully understandable) discussion is attached as "d3494_npe_writeup.html"...

> Move the setup of NormalizeResultSetNode into the NormalizeResultSetNode
> ------------------------------------------------------------------------
>
>                 Key: DERBY-3494
>                 URL: https://issues.apache.org/jira/browse/DERBY-3494
>             Project: Derby
>          Issue Type: Improvement
>          Components: SQL
>    Affects Versions: 10.4.0.0
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: d3494_npe_writeup.html, decompile.out, npe.sql
>
>
> In DERBY-3310 Dan suggested ...
> Setting up a NormalizeResultSetNode is spread over three locations, the class itself
(very little, it's almost acting like a C struct),
> the genNormalizeResultSetNode method and then copyLengthsAndTypesToSource. A good O-O
implementation would have
> the logic to create a NormalizeResultSetNode self-contained in NormalizeResultSetNode.
> Since the ResultColumnList of the original ResultSetNode correctly describes the desired
outcome, it's not clear to
> me why NormalizeResultSetNode can't just refer to the same list and use it for its processing.
They may be some chance
> that this would cause recursion at some point, where a NormalizeResultSetNode would think
it needed to be wrapped
> in a NormalizeResultSetNode since the types of its columns and expression don't match
(i.e. when it is handled as a regular ResultSetNode).
> I think moving the setup of a NormalizeResultSetNode into the class itself, so that its
inputs are just the ResultSetNode to wrap
> would help clear up the code, especially if comments were added indicating why certain
actions were being taken.
> I am separating this task out into a separate issue, so that it can be worked on independently
of DERBY-3310.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message