[
https://issues.apache.org/jira/browse/DERBY-4631?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mamta A. Satoor updated DERBY-4631:
-----------------------------------
Attachment: DERBY_4631_patch5_diff.txt
Hi Mike,
thanks for reviewing patch 4. Here are some comments to your feedback(Attaching
a new patch DERBY_4631_patch5_diff.txt which takes care of some of your
comments.)
1)ResultColumn:
********
Mike's comment - There is a lot of checking for instances here, is there anyway
to do this work in the affected nodes like HalfOuterJoinNode?
********
Yes, I am concerned about the instance checking too but I had researched into
putting the code in HalfOuterJoinNode and found that HalfOuterJoinNode does not
ever get to see the ResultColumns for the query and hence it has no way of
marking those ResultColumns as join columns.
********
Mike's comment - At end of ResultColumn changes why do you check and set for
one of the new fields and not the other?
********
I was trying to follow the existing code where the boolean kinds of fields are
first checked and then set to true if the check returned true.
rightOuterJoinUsingClause is a boolean field and hence I checked for the return
value and then set the new object's value to true. But joinResultSet is a
non-boolean field and hence I simply used it's value to set new object's
joinResultSet value. For clarity, I will go ahead and replace following
newResultColumn.setJoinResultset(getJoinResultSet());
with
if (getJoinResultSet() != null) {
newResultColumn.setJoinResultset(getJoinResultSet());
}
2)ResultColumnList.java:
********
Mike's comment - get rid of the commented out line of code that you fixed with
patch 4
********
Removed it.
********
Mike's comment - a comment in allExpressionsAreColumns explaining why returning
false for isRightOuterJoinUsingClause would be useful.
********
Added a comment.
********
Mike's comment - in mapSourceColumns() what does the -1 for right outer join
columns mean?
********
allExpressionsAreColumns() uses the -1 value set by mapSourceColumns() to
decide if there are columns which require special consideration during code
generation. When mapSourceColumns() assigns -1 to right outer join column,
allExpressionsAreColumns() will return false. This will allow Derby to later
generate code equivalent to COASLECE for right outer join columns(this code
generation happens in newly added code in ResultColumnList.generateCore.)
********
Mike's comment - typo - search for "righ "
********
Fixed it.
********
Mike's comment - would be nice to have comments for setJoinResultset and
setRightOuterJoinUsingClause, maybe something about what is expected to call
this routine and in what circumstances.
********
Added comments around for both those methods.
Additionally, took care of some indentation problems with the code.
> Wrong join column returned by right outer join with NATURAL or USING and
> territory-based collation
> --------------------------------------------------------------------------------------------------
>
> Key: DERBY-4631
> URL: https://issues.apache.org/jira/browse/DERBY-4631
> Project: Derby
> Issue Type: Bug
> Components: SQL
> Affects Versions: 10.6.1.0
> Reporter: Knut Anders Hatlen
> Assignee: Mamta A. Satoor
> Labels: derby_triage10_8
> Attachments: DERBY_4631_not_for_commit_patch1_diff.txt,
> DERBY_4631_not_for_commit_patch1_stat.txt,
> DERBY_4631_not_for_commit_patch2_diff.txt,
> DERBY_4631_not_for_commit_patch2_stat.txt, DERBY_4631_patch3_diff.txt,
> DERBY_4631_patch3_stat.txt, DERBY_4631_patch4_diff.txt,
> DERBY_4631_patch5_diff.txt
>
>
> SQL:2003 says that the join columns in a natural join or in a named
> columns join should be added to the select list by coalescing the
> column from the left table with the column from the right table.
> Section 7.7, <joined table>, syntax rules:
> > 1) Let TR1 be the first <table reference>, and let TR2 be the <table
> > reference> or <table factor> that is the second operand of the
> > <joined table>. Let RT1 and RT2 be the row types of TR1 and TR2,
> > respectively. Let TA and TB be the range variables of TR1 and TR2,
> > respectively. (...)
> and
> > 7) If NATURAL is specified or if a <join specification> immediately
> > containing a <named columns join> is specified, then:
> (...)
> > d) If there is at least one corresponding join column, then let SLCC
> > be a <select list> of <derived column>s of the form
> >
> > COALESCE ( TA.C, TB.C ) AS C
> >
> > for every column C that is a corresponding join column, taken in
> > order of their ordinal positions in RT1.
> For a right outer join, Derby doesn't use COALESCE(TA.C, TB.C), but
> rather just TB.C (the column in the right table) directly.
> This is in most cases OK, because COALESCE(TA.C, TB.C) = TB.C is an
> invariant in a right outer join. (Because TA.C is either NULL or equal
> to TB.C.)
> However, in a database with territory-based collation, equality
> between two values does not mean they are identical, especially now
> that the strength of the collator can be specified (DERBY-1748).
> Take for instance this join:
> ij> connect
> 'jdbc:derby:testdb;create=true;territory=en_US;collation=TERRITORY_BASED:SECONDARY';
> ij> create table big(x varchar(5));
> 0 rows inserted/updated/deleted
> ij> insert into big values 'A','B','C';
> 3 rows inserted/updated/deleted
> ij> create table small(x varchar(5));
> 0 rows inserted/updated/deleted
> ij> insert into small values 'b','c','d';
> 3 rows inserted/updated/deleted
> ij> select x, t1.x, t2.x, coalesce(t1.x, t2.x) from small t1 natural right
> outer join big t2;
> X |X |X |4
> -----------------------
> A |NULL |A |A
> B |b |B |b
> C |c |C |c
> 3 rows selected
> I believe that the expected result from the above query is that the
> first column should have the same values as the last column. That is,
> the first column should contain {'A', 'b', 'c'}, not {'A', 'B', 'C'}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
