[ 
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.

Reply via email to