You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Lluis Turro (JIRA)" <de...@db.apache.org> on 2006/01/27 16:35:32 UTC
[jira] Created: (DERBY-883) Date functions are 'syntax error' in
group by clause
Date functions are 'syntax error' in group by clause
----------------------------------------------------
Key: DERBY-883
URL: http://issues.apache.org/jira/browse/DERBY-883
Project: Derby
Type: Bug
Components: SQL
Versions: 10.1.2.1
Environment: JDK 1.5.0_05
Reporter: Lluis Turro
This select would return an error syntax on finding "(" after month if group by clause:
select idissue, month(creation), year(creation), count(distinct idissue)
where
....
group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
Re: Re: [jira] Commented: (DERBY-883) Enhance GROUP BY clause to support expressions instead of just column references.
Posted by Andrew McIntyre <mc...@gmail.com>.
On 8/9/06, Manish Khettry <ma...@yahoo.com> wrote:
> To build another patch, I updated my source tree.
> There was one conflict in BinaryOperatorNode (fairly
> trivial) but now the main line doesn't build. Is there
> something wrong in my environment?
>
> compile_tools_impl_mt:
> [javac] Compiling 1 source file to
> C:\derby\trunk\classes
> [javac]
> C:\derby\trunk\java\tools\org\apache\derby\impl\tools\ij\mtGrammar.j
> ava:387: cannot find symbol
> [javac] symbol : constructor
> SimpleCharStream(java.io.InputStream,java.lang
> .String,int,int)
> [javac] location: class
These errors are in parser classes generated by javacc. A clobber and
rebuild should fix the problem.
andrew
Re: [jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by Kristian Waagan <Kr...@Sun.COM>.
Manish Khettry wrote:
> To build another patch, I updated my source tree.
> There was one conflict in BinaryOperatorNode (fairly
> trivial) but now the main line doesn't build. Is there
> something wrong in my environment?
Hi Manish,
I'm able to build, so there might be something wrong in your
environment. I built trunk on Solaris and Linux.
Regards,
--
Kristian
>
> compile_tools_impl_mt:
> [javac] Compiling 1 source file to
> C:\derby\trunk\classes
> [javac]
> C:\derby\trunk\java\tools\org\apache\derby\impl\tools\ij\mtGrammar.j
> ava:387: cannot find symbol
> [javac] symbol : constructor
> SimpleCharStream(java.io.InputStream,java.lang
> .String,int,int)
> [javac] location: class
> org.apache.derby.impl.tools.ij.SimpleCharStream
> [javac] try { jj_input_stream = new
> SimpleCharStream(stream, encoding, 1
> , 1); } catch(java.io.UnsupportedEncodingException e)
> { throw new RuntimeExcepti
> on(e); }
> [javac] ^
> [javac]
> C:\derby\trunk\java\tools\org\apache\derby\impl\tools\ij\mtGrammar.j
> ava:400: cannot find symbol
> [javac] symbol : method
> ReInit(java.io.InputStream,java.lang.String,int,int
> )
> [javac] location: class
> org.apache.derby.impl.tools.ij.SimpleCharStream
> [javac] try { jj_input_stream.ReInit(stream,
> encoding, 1, 1); } catch(ja
> va.io.UnsupportedEncodingException e) { throw new
> RuntimeException(e); }
> [javac] ^
> [javac] 2 errors
>
> At rev 430199
>
> --- Andrew McIntyre <mc...@gmail.com> wrote:
>
>> On 8/9/06, Daniel John Debrunner (JIRA)
>> <de...@db.apache.org> wrote:
>>> [
> http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12427050
>> ]
>>> Daniel John Debrunner commented on DERBY-883:
>>> ---------------------------------------------
>>>
>>> Darn - this patch doesn't apply cleanly for me,
>> about 8-10 files have conflicts, otherwise I would
>> have committed it.
>>
>> Don't forget, svn is smarter than patch when it
>> comes to conflicts.
>> Usually you can go back to the revision the patch
>> was created at,
>> apply the patch, and roll forward to the head and
>> svn will merge the
>> changes properly. So, try:
>>
>> svn up -r 425465
>> patch -p0 < 883.patch5.txt
>> svn up
>>
>> andrew
>>
>
>
> __________________________________________________
> Do You Yahoo!?
> Tired of spam? Yahoo! Mail has the best spam protection around
> http://mail.yahoo.com
Re: [jira] Commented: (DERBY-883) Enhance GROUP BY clause to support expressions instead of just column references.
Posted by Manish Khettry <ma...@yahoo.com>.
To build another patch, I updated my source tree.
There was one conflict in BinaryOperatorNode (fairly
trivial) but now the main line doesn't build. Is there
something wrong in my environment?
compile_tools_impl_mt:
[javac] Compiling 1 source file to
C:\derby\trunk\classes
[javac]
C:\derby\trunk\java\tools\org\apache\derby\impl\tools\ij\mtGrammar.j
ava:387: cannot find symbol
[javac] symbol : constructor
SimpleCharStream(java.io.InputStream,java.lang
.String,int,int)
[javac] location: class
org.apache.derby.impl.tools.ij.SimpleCharStream
[javac] try { jj_input_stream = new
SimpleCharStream(stream, encoding, 1
, 1); } catch(java.io.UnsupportedEncodingException e)
{ throw new RuntimeExcepti
on(e); }
[javac] ^
[javac]
C:\derby\trunk\java\tools\org\apache\derby\impl\tools\ij\mtGrammar.j
ava:400: cannot find symbol
[javac] symbol : method
ReInit(java.io.InputStream,java.lang.String,int,int
)
[javac] location: class
org.apache.derby.impl.tools.ij.SimpleCharStream
[javac] try { jj_input_stream.ReInit(stream,
encoding, 1, 1); } catch(ja
va.io.UnsupportedEncodingException e) { throw new
RuntimeException(e); }
[javac] ^
[javac] 2 errors
At rev 430199
--- Andrew McIntyre <mc...@gmail.com> wrote:
> On 8/9/06, Daniel John Debrunner (JIRA)
> <de...@db.apache.org> wrote:
> > [
>
http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12427050
> ]
> >
> > Daniel John Debrunner commented on DERBY-883:
> > ---------------------------------------------
> >
> > Darn - this patch doesn't apply cleanly for me,
> about 8-10 files have conflicts, otherwise I would
> have committed it.
>
> Don't forget, svn is smarter than patch when it
> comes to conflicts.
> Usually you can go back to the revision the patch
> was created at,
> apply the patch, and roll forward to the head and
> svn will merge the
> changes properly. So, try:
>
> svn up -r 425465
> patch -p0 < 883.patch5.txt
> svn up
>
> andrew
>
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
Re: [jira] Commented: (DERBY-883) Enhance GROUP BY clause to support expressions instead of just column references.
Posted by Andrew McIntyre <mc...@gmail.com>.
On 8/9/06, Daniel John Debrunner (JIRA) <de...@db.apache.org> wrote:
> [ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12427050 ]
>
> Daniel John Debrunner commented on DERBY-883:
> ---------------------------------------------
>
> Darn - this patch doesn't apply cleanly for me, about 8-10 files have conflicts, otherwise I would have committed it.
Don't forget, svn is smarter than patch when it comes to conflicts.
Usually you can go back to the revision the patch was created at,
apply the patch, and roll forward to the head and svn will merge the
changes properly. So, try:
svn up -r 425465
patch -p0 < 883.patch5.txt
svn up
andrew
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Yip Ng (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12445990 ]
Yip Ng commented on DERBY-883:
------------------------------
Some discussions regarding GROUP BY with expression in dev-list:
http://www.nabble.com/Functions-in-GROUP-BY-expressions--%28related-to-DERBY-883%29-tf2517296.html
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL, Documentation
> Affects Versions: 10.1.2.1, 10.2.1.6
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.6
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12432263 ]
Manish Khettry commented on DERBY-883:
--------------------------------------
I think the issue can be closed.
I'm not sure if we want to allow the use of grouping expressions in the having clause; i.e.
select c+1, count(*)
from t
group by c+1
having c+1 > 1;
I only have access to mysql, which doesn't allow this but does let you do
select c, count(*)
from t
group by c
having c > 1;
If we do, then we need another bug or a subtask of this issue.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.3.0.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Satheesh Bandaram (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12412227 ]
Satheesh Bandaram commented on DERBY-883:
-----------------------------------------
Thanks for making progress on this important new functionality, Manish.
> 1. In terms of syntax, do we allow expressions in the group by list or positional parameters, or both?
>
> select tomonth(creationdt), toyear(creationdt), count(*)
> from bugs
> group by 1, 2;
I have seen positional parameters for ORDER BY expressions, not typically used in GROUP BY. Looking at both DB2 and Oracle documentation, it seems neither support positional parameters.
> An implementation question on this note-- does the language code have a way of looking
> at two expressions (ValueNode?) and checking to see if they are equivalent? We'll need
> some way of doing this to match an expression in the group by list to an expression
> in the select list right?
Correct. Don't think there is any existing expression matching to compare two expressions. DB2 docs discuss how group by expressions are matched in SQL reference manual. (Page 484: ftp://ftp.software.ibm.com/ps/products/db2/info/vr82/pdf/en_US/db2s1e81.pdf)
> 2. I assume that an expression in a group by list must appear in the select list without
> aggregation right? Is this an error?
>
> select x+1, x+2, sum(y)
> from test
> group by x
NO... This is a valid query. See the reference I provided above.
> 3. What do we do with duplicates? i.e.
>
> select x+1, x+1, sum(y)
> from test
> group by x+1, x+1;
>
> Is this an error? The current implementation throws an error if the same column
> occurs more than once in the group by list.
I am not sure why Derby currently considers this an error... Looking at the code, it seems it may be looking for ambiguous column references (like 'x' being part of two different tables in from_list), which makes sense, but not sure why duplicate references should be prevented.
> Is there a standard somewhere which I should consult before trying to nail down the functionality?
Unfortunately, NO.... SQL 2003 seems to allow only column references in GROUP BY clause. But both DB2 and Oracle allow expressions in GROUP BY list and likely allowed by other database vendors too. You could use either DB2 or Oracle docs to understand how this functionality is defined there. Much easier to read these docs than confusing SQL 2003 spec.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: New Feature
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assignee: Manish Khettry
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Lluis Turro (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12411920 ]
Lluis Turro commented on DERBY-883:
-----------------------------------
Manish, it would be great supporting positional parameters. So I vote for both ;-)
Duplicates in group by clause make no sense, is an error.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: New Feature
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assignee: Manish Khettry
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Satheesh Bandaram (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Satheesh Bandaram updated DERBY-883:
------------------------------------
Summary: Enhance GROUP BY clause to support expressions instead of just column references. (was: Date functions are 'syntax error' in group by clause)
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: New Feature
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12430668 ]
Daniel John Debrunner commented on DERBY-883:
---------------------------------------------
I spent some time on this patch and have it merged to trunk - will run tests and then commit.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Satheesh Bandaram (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12378688 ]
Satheesh Bandaram commented on DERBY-883:
-----------------------------------------
Thanks for picking this up, Manish. I also have itch to see this completed soon, so let me know if and how I can help.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: New Feature
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assignee: Manish Khettry
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Assigned: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Manish Khettry reassigned DERBY-883:
------------------------------------
Assign To: Manish Khettry
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: New Feature
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assignee: Manish Khettry
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Manish Khettry updated DERBY-883:
---------------------------------
Attachment: 883.patch4.txt
I found a problem in one of my changes. Also got rid of a lot of white space only diffs.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Manish Khettry updated DERBY-883:
---------------------------------
Attachment: 883.patch3.txt
incorporate feedback from code review.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Resolved: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Andrew McIntyre (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Andrew McIntyre resolved DERBY-883.
-----------------------------------
Resolution: Fixed
Marking this resolved. Opened DERBY-2170 to cover the documentation update.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL, Documentation
> Affects Versions: 10.1.2.1, 10.2.1.6
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.6
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Manish Khettry updated DERBY-883:
---------------------------------
Attachment: 883.patch.txt
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12378466 ]
Manish Khettry commented on DERBY-883:
--------------------------------------
I can look into the fix for 134 and see if the work done there can be used for this one; unless someone else (Tomohito perhaps) wants to look into this first.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: New Feature
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12427050 ]
Daniel John Debrunner commented on DERBY-883:
---------------------------------------------
Darn - this patch doesn't apply cleanly for me, about 8-10 files have conflicts, otherwise I would have committed it.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Kristian Waagan (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12431594 ]
Kristian Waagan commented on DERBY-883:
---------------------------------------
Are there any plans to merge 883.patch6.txt (revision 437070) to 10.2?
I found out that it fixes the NPE with COALESCE (DERBY-1774). I don't know why.
I tried to merge by applying the diff from 'svn diff -r 437069:437070', and that worked fine.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.3.0.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12431598 ]
Daniel John Debrunner commented on DERBY-883:
---------------------------------------------
"worked fine" as in merged without errors, or "worked fine" as in derbyall passed on the branch wih the merge changes?
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.3.0.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Kristian Waagan (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12431994 ]
Kristian Waagan commented on DERBY-883:
---------------------------------------
derbyall with the patch 6 merged ran 673 tests on Solaris10, Sun JDK 1.5.
1 failure, which I think might be caused by my test enviroment; Upgrade_10_1_10_2.java.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.3.0.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Manish Khettry updated DERBY-883:
---------------------------------
Attachment: 883.patch5.txt
This patch addresses issues raised by Army and Yip.
1. Flag error for aggregate in group by list. Changes to sqlgrammar.jj, SQLState.java and messages_en.properties. Other language files need to be changed too.
2. Remove unused SQL state (42Y19).
3. Since the junit file (GroupByExpressionTest.java) is new, use spaces instead of tabs to indent. Fix indentation for long lines.
4. Remove commented out code.
5. Add a few comments suggested by Army.
6. Test ternary operator node#isEquivalent in test case.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: [jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by Mike Matrigali <mi...@sbcglobal.net>.
Daniel John Debrunner wrote:
> Kristian Waagan (JIRA) wrote:
>
>
>> [ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12431617 ]
>>
>>Kristian Waagan commented on DERBY-883:
>>---------------------------------------
>>
>>The former; merged without errors.
>>Was this a general question, or is there reason to believe a merge will cause problems?
>
>
> I think a general question. If the answer had been "passed derbyall/some
> tests" I would have tried to find time to commit it today. Just good to
> be clear on what was done.
>
I want to second this, any help the community can provide in reviewing
and running tests can help the committers out. I have been trying to
move patches along in the last few weeks, but especially in areas out
of my expertise I tend to want to run a full test run which effectively
means for me a day for each commit - if I don't need to run tests for
something else I am working on.
For a merge from trunk to 10.2 I would very likely be much more
comfortable running less tests before commit if someone in the
community let me know
they had already run the merge and run all the tests.
>
>>I'll start a derbyall tonight, and report back tomorrow.
>
>
> Great, we'll see what the results are.
>
> Thanks,
> Dan.
>
>
>
>
Re: [jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by Daniel John Debrunner <dj...@apache.org>.
Kristian Waagan (JIRA) wrote:
> [ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12431617 ]
>
> Kristian Waagan commented on DERBY-883:
> ---------------------------------------
>
> The former; merged without errors.
> Was this a general question, or is there reason to believe a merge will cause problems?
I think a general question. If the answer had been "passed derbyall/some
tests" I would have tried to find time to commit it today. Just good to
be clear on what was done.
> I'll start a derbyall tonight, and report back tomorrow.
Great, we'll see what the results are.
Thanks,
Dan.
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Kristian Waagan (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12431617 ]
Kristian Waagan commented on DERBY-883:
---------------------------------------
The former; merged without errors.
Was this a general question, or is there reason to believe a merge will cause problems?
I'll start a derbyall tonight, and report back tomorrow.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.3.0.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Date functions are 'syntax error' in
group by clause
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Daniel John Debrunner updated DERBY-883:
----------------------------------------
type: New Feature (was: Bug)
Seems like supporting expressions in the GROUP BY clause is similar to the improvement made to support expressions in the ORDER BY clause that Tomohito did a while ago.
Can we use the knowledge/implementation details from the ORDER BY work to do the GROUP BY improvement?
> Date functions are 'syntax error' in group by clause
> ----------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: New Feature
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Yip Ng (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12424010 ]
Yip Ng commented on DERBY-883:
------------------------------
Hi Manish:
I am also reviewing your DERBY-883 patch. Since the group by clause has been relaxed to allow expressions now, we'll need to return a proper SQLSTATE for invalid aggregate functions use in group by clause. i.e.: The following test is throwing a NPE:
ij> create table t1 (c1 int, c2 varchar(10));
0 rows inserted/updated/deleted
ij> insert into t1 values (1, 'data1'), (2, 'data1'), (3, 'data2');
3 rows inserted/updated/deleted
ij> select sum(c1), 1 from t1 group by sum(c1);
ERROR XJ001: Java exception: ': java.lang.NullPointerException'.
ij> select * from t1;
ERROR 40XT0: An internal error was identified by RawStore module.
I'll post additional comments after I have review all the changes.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "A B (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12423742 ]
A B commented on DERBY-883:
---------------------------
Review comments incorporated into version 3 and 4 patches can be found here:
http://article.gmane.org/gmane.comp.apache.db.derby.devel/24480
Review comments on the phase 4 patch can be found here:
http://article.gmane.org/gmane.comp.apache.db.derby.devel/24708
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12430681 ]
Manish Khettry commented on DERBY-883:
--------------------------------------
no functional changes, so if you've fixed up patch 6 to work, then go with it.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Yip Ng (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12427044 ]
Yip Ng commented on DERBY-883:
------------------------------
Thanks for continuing to improve the patch, Manish. The changes look good to me and I think this patch is ready for commit. +1
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Date functions are 'syntax error' in
group by clause
Posted by "Mike Perham (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12374772 ]
Mike Perham commented on DERBY-883:
-----------------------------------
This bug makes unit testing our data warehousing queries essentially impossible without modifying our actual schema. This needs to be bumped from major to critical IMO.
> Date functions are 'syntax error' in group by clause
> ----------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: Bug
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Daniel John Debrunner updated DERBY-883:
----------------------------------------
Derby Info: (was: [Patch Available])
Patch 883.patch6.txt applied to trunk with changes below as revision 437070.
0) Merged by hand to latest codeline, only one real clash due to other added methods on ColumnReference
1) Licence header changed for new files to match new policy
2) Adapt patch for new location of BaseJDBCTestCase
3) Cleanup of new GroupByExpressionTest to use new single connection model provided by BaseJDBCTestCase and not have a single static Connection field. Static fields will cause a problem if multiple tests are run concurrently.
4) Minor cleanup of new assert in BaseJDBCTestCase
Thanks Manish, especially for sticking with it. :-)
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Manish Khettry updated DERBY-883:
---------------------------------
Attachment: 883.patch6.txt
Minor correction to previous patch.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12430676 ]
Daniel John Debrunner commented on DERBY-883:
---------------------------------------------
Sorry I was on the way to the bus when I asked the question so it was brief.
I meant is the functionality provided by patchs 6 and 7 the same, did you fix any bugs etc. between 6 and 7 or was it just adjusting to the codeline changes. I've already started tests with my merge of 6 so I'll keep with that unless there's added benefit to patch 7.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Lluis Turro (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12411924 ]
Lluis Turro commented on DERBY-883:
-----------------------------------
About question 2. Results would not change but would be a shortcut. If you ask me I would tell you that reads better 'group by x'. Remember you can also group fields in where clause and they don't need to appear in the select list.
select sum(quantity)
from client
group by clientId
I got a list of 'quantity' where to perform operations, but don't care which client provides each element.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: New Feature
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assignee: Manish Khettry
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Closed: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Dyre Tjeldvoll (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dyre Tjeldvoll closed DERBY-883.
--------------------------------
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: https://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: Documentation, SQL
> Affects Versions: 10.1.2.1, 10.2.1.6
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assignee: Manish Khettry
> Fix For: 10.2.1.6
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-883) Date functions are 'syntax error' in
group by clause
Posted by "Matthias Ohlemeyer (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12374884 ]
Matthias Ohlemeyer commented on DERBY-883:
------------------------------------------
If I read the documentation correctly (Reference Manual) then your SQL statement is simply not allowed in Derby; it says
GROUP BY column-Name [ , column-Name ] *
where column-Name is a simple column name or a correlation; in my experiences correlations only work when they refer to a simple column.
But IMO you are still right: Derby is missing an essential feature here; instead of the above something like
GROUP BY SelectItem [ , SelectItem ] *
would be more in line with what other DBs have to offer; it should be possible to use arbitrary select items from the SELECT clause in the GROUP BY clause. This "shortcoming" has made it close to impossible to port a complex Oracle based application to Derby.
But since the documentation seems to be clear, I think this "bug" should be reclassified as a "New Feature" or "Improvement"; for me the priority cannot be high enough, it is "Major" at least and of course "Critical" for my application, but I'm not sure this can be generalized.
> Date functions are 'syntax error' in group by clause
> ----------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: Bug
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12427381 ]
Manish Khettry commented on DERBY-883:
--------------------------------------
Dan could you try the last patch file attached with this bug?
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12402467 ]
Manish Khettry commented on DERBY-883:
--------------------------------------
I have a few questions on this, largely functional.
1. In terms of syntax, do we allow expressions in the group by list or positional parameters, or both?
select tomonth(creationdt), toyear(creationdt), count(*)
from bugs
group by 1, 2;
or
group by tomonth(creationdt), toyear(creationdt);
An implementation question on this note-- does the language code have a way of looking at two expressions (ValueNode?) and checking to see if they are equivalent? We'll need some way of doing this to match an expression in the group by list to an expression in the select list right?
2. I assume that an expression in a group by list must appear in the select list without aggregation right? Is this an error?
select x+1, x+2, sum(y)
from test
group by x
;
3. What do we do with duplicates? i.e.
select x+1, x+1, sum(y)
from test
group by x+1, x+1;
Is this an error? The current implementation throws an error if the same column occurs more than once in the group by list.
Is there a standard somewhere which I should consult before trying to nail down the functionality?
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: New Feature
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assignee: Manish Khettry
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Yip Ng (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Yip Ng updated DERBY-883:
-------------------------
Component/s: Documentation
Fix Version/s: 10.2.1.6
(was: 10.3.0.0)
This feature is integrated into 10.2, so the fix version should be 10.2.1.6 and not 10.3.0.0.
Including the Documentation component since it needs to be documented in 10.2 reference manual - GROUP BY clause.
http://db.apache.org/derby/docs/10.2/ref/rrefsqlj32654.html
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL, Documentation
> Affects Versions: 10.1.2.1, 10.2.1.6
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.6
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12432257 ]
Daniel John Debrunner commented on DERBY-883:
---------------------------------------------
Is this issue resolved now?
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.3.0.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12429218 ]
Daniel John Debrunner commented on DERBY-883:
---------------------------------------------
Manish, can you fix up the patch so it works with the current state of the trunk. BaseJDBCTestCase has moved and the changes to sqlgrammar.jaa and CoalesceFunctionNode do not apply cleanly. If you can get this done this weekend then I will apply it.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Date functions are 'syntax error' in
group by clause
Posted by "Lluis Turro (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12364222 ]
Lluis Turro commented on DERBY-883:
-----------------------------------
Correct SQL statement should be:
select status, month(creation), year(creation), count(distinct idissue)
where
....
group by status, month(creation), year(creation)
The fact remains the same, but previous was mistaking.
> Date functions are 'syntax error' in group by clause
> ----------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Type: Bug
> Components: SQL
> Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12430671 ]
Daniel John Debrunner commented on DERBY-883:
---------------------------------------------
Were there any function changes between patch 6 and 7?
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12428745 ]
Daniel John Debrunner commented on DERBY-883:
---------------------------------------------
Sorry I missed this was available, I will look at committing this.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: [jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by Daniel John Debrunner <dj...@apache.org>.
Army wrote:
> **** Nit-picking: Changes that you don't *have* to do but that might
> help in cleaning up the patch.
>
> A. Unnecessary white-space changes?
+1 to avoiding such changes.
> In SelectNode.java:
>
> - VerifyAggregateExpressionsVisitor visitor =
> - new VerifyAggregateExpressionsVisitor(groupByList);
> + VerifyAggregateExpressionsVisitor visitor =
> + new VerifyAggregateExpressionsVisitor(groupByList);
> resultColumns.accept(visitor);
White space changes like this make it extra work on the reviewers. Is it
really just white space change or is there some minor change in there
that they need to be concerned about. Looking at such changes makes me
go cross-eyed as I try to determine if the lines are the same or not.
Thanks,
Dan.
Re: [jira] Updated: (DERBY-883) Enhance GROUP BY clause to support expressions instead of just column references.
Posted by Manish Khettry <ma...@gmail.com>.
On 7/26/06, Army <qo...@gmail.com> wrote:
>
> Hi Manish,
>
> Thanks for addressing my comments. The patch is looking better
> now--especially
> with the whitespace issues in version 4. Some follow-up comments below.
>
> Manish Khettry wrote:
> >> 1) TernarnyOpNode.isEquivalent (): I think we need to check "receiver"
> >> equivalence as well, in addition to left and right operands.
> >
> > yes, you are right on this one. The new patch fixes this.
>
> Thanks for fixing this. If possible, could you add a test case to your
> JUnit
> test to demonstrate? Could just be the query that I posted in my previous
> comments.
I have added a couple of queries to make sure that
TernaryOpNode#isEquivalent is tested.
> Actually the existing error message no longer makes sense with group by
> > expressions. For example, the line of code that you point to, also
> catches
> > errors like this:
> > select c1+1, avg(c2) from test group by c1+10;
> > Does the existing error message make sense for this query?
>
> Not with this query, no. But two things to note here:
>
> 1) Before your patch, if you replace "group by c1+10" with "group by c2"
> you'll
> get error 42Y36 saying that column reference C1 is invalid, even though
> it's
> actually part of an expression ("c1+1")--so that type of behavior *does*
> already
> exist, even if it doesn't really "make sense". If you change the SQLSTATE
> and
> error message for this case, the result is a change (albeit a pretty small
> one)
> in customer apps, which is really the issue here.
>
> 2) The key expression in my previous comment was "in cases like this",
> where
> "like this" meant queries where the existing error message *does* make
> sense.
>
> For example, with the following query it makes sense to include the column
> reference because we know what it is:
>
> ij> select c1 from test group by c2;
> ERROR 42Y36: Column reference 'C1' is invalid. For a SELECT list with a
> GROUP
> BY, the list may only contain grouping columns and valid aggregate
> expressions.
>
> And that's what we'll get before your patch. But after your patch we a)
> get a
> different SQLSTATE and b) lose the column reference name:
>
> ij> select c1 from test group by c2;
> ERROR 42Y30: The SELECT list of a grouped query contains at least one
> invalid
> expression. If a SELECT list has a GROUP BY, the list may only contain
> valid
> grouping expressions and valid aggregate expressions.
>
> So now we've changed the way an existing query behaves (by returning a
> different
> SQLSTATE)--which (I think?) means we need to mark this as "Existing
> Application
> Impact"--*and* we're now providing the user with less information.
I see your point but I would prefer to keep it this way, first because the
error message in question 42Y36-- even though it gives more information, is
misleading; specifically this phrase: "the list may only contain grouping
columns .....". I also don't like error handling for a subset of the cases
taking recourse to instanceof. I'll throw it up to the community to weigh
in.
I can see that it would be useful to point out which expression is invalid
as opposed to saying ".. atleast one invalid expression....". If the
community wants this, then I'd like to be done with this patch and file
another issue to specifically improve the error handling of this case.
If it's possible to figure out when we have a simple column reference (c1)
> verses an expression (c1+1) and to throw a corresponding error, I think
> that
> would be ideal. But if you decide that it's not possible or not worth it
> to
> preserve the error for queries with simple column references, then feel
> free to
> leave this as it is. If that's your decision, though, I think you should
> ask
> derby-dev if a different SQLSTATE for the same query counts as "Existing
> Application Impact", and if so, mark this issue as appropriate.
>
> >> + return (operator.equals(other.operator)
> >> + || (operand == other.operand)
> >> + || ((operand != null) && operand.isEquivalent(other)));
> >
> > If you look at bindUnaryOperator for UnaryOperatorNode, you will see
> that
> > it is possible for operand to be null; not so in BinaryOperatorNode.
>
> So the assumption here is that if (operand == other.operand) then both
> operands
> are null? If so, a simple comment saying that could be helpful.
OK. I think the code speaks clearly for itself (the second part of the ||
checks for != null before calling isEquivalent) but I've added the comment.
I like to add comments if they really clarify something that the code may
not. Usually the code changes and the comments don't and thats worse than no
comments at all. Those XP guys were onto something!
> In GroupByList.java:
> <snip code snippet>
> >
> > That is a good question-- this bit of code add invisible column
> references
> > when a column reference in the group by list, is not in the projection
> > list;
> > i.e.
> >
> > select c2, sum(c3) from test group by c1;
> >
> > will add c1 to the projection list as a generated column. I'm not sure
> if
> > you want to do the same kind of rewrite for expressions. I thought this
> > wasn't worth doing but I'm not sure anymore.
>
> I just tried this query with and without your patch and it fails in both
> cases
> with error 42Y36/42Y30. So I guess I'm a little confused as to why Derby
> would
> add "c1" to the projection list and what good it does us?
Sorry, wrong query, try
select c2, sum(c3) from test group by c1,c2;
If you're not sure whether or not it's worth adding equivalent logic for
> expressions, do you plan to follow-up with this later? If not, a comment
> explaining what you have found (or what your uncertainties are) might help
> other
> developers down the road (esp. if a bug comes up in this area later).
I'll add a comment. My preference would be to leave as is and evaluate it
later.
> > I disagree with you here-- when a contributor has spent a considerable
> > amount of time understanding and making a lot of changes to a class, it
> > makes sense to remove dead code at the same time. If you don't, stuff
> like
> > this will likely get ignored or slip through the cracks, imho.
>
> I didn't mean to say that the changes were not useful nor that they
> shouldn't be
> committed; just that it might make it easier to track "cleanup" vs
> "functional"
> changes by filing a separate Jira issue to remove the dead code and
> posting an
> isolated patch to that issue. The changes will still go in--no slipping
> through
> the cracks--but they will go in as part of their *own* Jira commit, which
> seems
> cleaner to me.
>
> In any event, that was just a suggestion. No need to let this prevent the
>
> commit of the patch.
OK, I see your point although the overhead of a new jira issue, code reviews
and such is a bit high, imho.
> 7) There are a couple of places where you have commented lines out instead
> >
> > yes, you're right. I've removed it.
>
> There are still a couple of these lingering:
I got rid of both of them.
>> A. Unnecessary white-space changes?
> >
> > Atleast some of these were done to improve existing indentation. Next
> time,
> > I'll resist the urge to improve readability.
>
> Thanks for cleaning a bunch of these whitespace diffs up in the version
> four
> patch. That said, there are still quite a few white-space changes
> left--did you
> intentionally leave those in? Ex.
>
> GroupByNode.java:
>
> @@ -470,10 +489,10 @@
> ** bottom project restrict.
> */
> newRC = (ResultColumn) getNodeFactory().getNode(
> -
> C_NodeTypes.RESULT_COLUMN,
> -
> "##aggregate result",
> -
> aggregate.getNewNullResultExpression(),
> -
> getContextManager());
> + C_NodeTypes.RESULT_COLUMN,
> + "##aggregate result",
> +
> aggregate.getNewNullResultExpression (),
> + getContextManager());
> newRC.markGenerated();
> newRC.bindResultColumnToExpression();
> bottomRCL.addElement (newRC);
Maybe one or two slipped by-- this one does reduce the indentation and its
possible it was going past the right margin and I decided to fix it. Some of
the others really served no purpose and I got rid of them.
1 -- SubstituteExpressionVisitor.java:
>
> * The copyright says 1998, 2004; I think this is supposed to just say
> 2006
> since that's when the file was created.
OK.
2 -- I see that in version 4 of the patch you updated UnaryOperatorNode to
> have
> the following logic in isEquivalent():
>
> + UnaryOperatorNode other = (UnaryOperatorNode)o;
> + return (operator.equals(other.operator) &&
> + ((operand == other.operand)||
> + ((operand != null) && operand.isEquivalent(
> other.operand))));
>
> Did you become aware of this problem because of some test(s) that you were
> running? If so, it'd be great if you could add the actual test case(s) to
> your
> JUnit test for regression purposes. The more tests there are to check for
> these
> kinds of subtleties, the better...
Not tests, just visual inspection. I was going over the email exchange when
it hit me. I thought about tests and the only one case where the operand is
null is count(*)-- since isEquivalent will only be called for grouping
expressions, we can never exercise this particular branch of the
conditional.
And finally:
>
> > Manish Khettry updated DERBY-883:
> > ---------------------------------
> >
> > Attachment: 883.patch4.txt
> >
> > I found a problem in one of my changes. Also got rid of a lot of
> > white space only diffs.
>
> In the future it would be helpful if you could say what changed between
> versions
> of the patch, so that reviewers don't have to scour the patches to try to
> figure
> out what changed. I managed to find out what "a problem" meant in this
> case,
> but it's generally a good idea to explicitly state the problem/changes in
> the
> comments. Makes my life as a reviewer easier :)
OK. I will attach a new patch shortly incorporating your comments and the
NPE that Yip found. Also, just as an fyi-- I will be away all of next week,
so if you have additional comments, I can only get to them, the second week
of august.
m
Re: [jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by Army <qo...@gmail.com>.
Hi Manish,
Thanks for addressing my comments. The patch is looking better now--especially
with the whitespace issues in version 4. Some follow-up comments below.
Manish Khettry wrote:
>> 1) TernarnyOpNode.isEquivalent(): I think we need to check "receiver"
>> equivalence as well, in addition to left and right operands.
>
> yes, you are right on this one. The new patch fixes this.
Thanks for fixing this. If possible, could you add a test case to your JUnit
test to demonstrate? Could just be the query that I posted in my previous comments.
> Actually the existing error message no longer makes sense with group by
> expressions. For example, the line of code that you point to, also catches
> errors like this:
> select c1+1, avg(c2) from test group by c1+10;
> Does the existing error message make sense for this query?
Not with this query, no. But two things to note here:
1) Before your patch, if you replace "group by c1+10" with "group by c2" you'll
get error 42Y36 saying that column reference C1 is invalid, even though it's
actually part of an expression ("c1+1")--so that type of behavior *does* already
exist, even if it doesn't really "make sense". If you change the SQLSTATE and
error message for this case, the result is a change (albeit a pretty small one)
in customer apps, which is really the issue here.
2) The key expression in my previous comment was "in cases like this", where
"like this" meant queries where the existing error message *does* make sense.
For example, with the following query it makes sense to include the column
reference because we know what it is:
ij> select c1 from test group by c2;
ERROR 42Y36: Column reference 'C1' is invalid. For a SELECT list with a GROUP
BY, the list may only contain grouping columns and valid aggregate expressions.
And that's what we'll get before your patch. But after your patch we a) get a
different SQLSTATE and b) lose the column reference name:
ij> select c1 from test group by c2;
ERROR 42Y30: The SELECT list of a grouped query contains at least one invalid
expression. If a SELECT list has a GROUP BY, the list may only contain valid
grouping expressions and valid aggregate expressions.
So now we've changed the way an existing query behaves (by returning a different
SQLSTATE)--which (I think?) means we need to mark this as "Existing Application
Impact"--*and* we're now providing the user with less information.
If it's possible to figure out when we have a simple column reference (c1)
verses an expression (c1+1) and to throw a corresponding error, I think that
would be ideal. But if you decide that it's not possible or not worth it to
preserve the error for queries with simple column references, then feel free to
leave this as it is. If that's your decision, though, I think you should ask
derby-dev if a different SQLSTATE for the same query counts as "Existing
Application Impact", and if so, mark this issue as appropriate.
>> + return (operator.equals(other.operator)
>> + || (operand == other.operand)
>> + || ((operand != null) && operand.isEquivalent(other)));
>
> If you look at bindUnaryOperator for UnaryOperatorNode, you will see that
> it is possible for operand to be null; not so in BinaryOperatorNode.
So the assumption here is that if (operand == other.operand) then both operands
are null? If so, a simple comment saying that could be helpful.
> In GroupByList.java:
<snip code snippet>
>
> That is a good question-- this bit of code add invisible column references
> when a column reference in the group by list, is not in the projection
> list;
> i.e.
>
> select c2, sum(c3) from test group by c1;
>
> will add c1 to the projection list as a generated column. I'm not sure if
> you want to do the same kind of rewrite for expressions. I thought this
> wasn't worth doing but I'm not sure anymore.
I just tried this query with and without your patch and it fails in both cases
with error 42Y36/42Y30. So I guess I'm a little confused as to why Derby would
add "c1" to the projection list and what good it does us?
If you're not sure whether or not it's worth adding equivalent logic for
expressions, do you plan to follow-up with this later? If not, a comment
explaining what you have found (or what your uncertainties are) might help other
developers down the road (esp. if a bug comes up in this area later).
> Most if not all of the routines which are deleted were made redundant by
> this fix. For example, getColumnReference in GroupByColumn is not only
> unused it makes no sense with group by expressions. I think its good to get
> rid of this code but if you want, I'd be perfectly willing to add dead code
> back in the source tree.
I guess it wasn't clear to me how all of the deleted methods were made redundant
by *this* fix. In your comment when you posted the patch you just said "unused
methods zapped", which sounded to me like you just happened to notice that the
methods were unused and therefore deleted them. That kind of spontaneous
"cleanup" is, like the whitespace changes, something that should be
posted/committed as a separate issue. If, however, the methods are relevant and
useful before your changes but redundant and deletable after your changes, then
it's fine to have them in with this patch.
> I disagree with you here-- when a contributor has spent a considerable
> amount of time understanding and making a lot of changes to a class, it
> makes sense to remove dead code at the same time. If you don't, stuff like
> this will likely get ignored or slip through the cracks, imho.
I didn't mean to say that the changes were not useful nor that they shouldn't be
committed; just that it might make it easier to track "cleanup" vs "functional"
changes by filing a separate Jira issue to remove the dead code and posting an
isolated patch to that issue. The changes will still go in--no slipping through
the cracks--but they will go in as part of their *own* Jira commit, which seems
cleaner to me.
In any event, that was just a suggestion. No need to let this prevent the
commit of the patch.
> 7) There are a couple of places where you have commented lines out instead
>
> yes, you're right. I've removed it.
There are still a couple of these lingering:
GroupByColumn.java:
public String getColumnName()
{
- return colRef.getColumnName();
+ //return colRef.getColumnName();
+ return columnExpression.getColumnName();
ColumnReference.java:
+ protected boolean isEquivalent(ValueNode o) throws StandardException
+ {
+ if (!isSameNodeType(o)) {
+ return false;
+ }
+ ColumnReference other = (ColumnReference)o;
+ return (tableNumber == other.tableNumber
+ && columnName.equals(other.getColumnName()));
+ //return source.isEquivalent(other.source);
+ }
>> A. Unnecessary white-space changes?
>
> Atleast some of these were done to improve existing indentation. Next time,
> I'll resist the urge to improve readability.
Thanks for cleaning a bunch of these whitespace diffs up in the version four
patch. That said, there are still quite a few white-space changes left--did you
intentionally leave those in? Ex.
GroupByNode.java:
@@ -470,10 +489,10 @@
** bottom project restrict.
*/
newRC = (ResultColumn) getNodeFactory().getNode(
- C_NodeTypes.RESULT_COLUMN,
- "##aggregate result",
- aggregate.getNewNullResultExpression(),
- getContextManager());
+ C_NodeTypes.RESULT_COLUMN,
+ "##aggregate result",
+ aggregate.getNewNullResultExpression(),
+ getContextManager());
newRC.markGenerated();
newRC.bindResultColumnToExpression();
bottomRCL.addElement(newRC);
>> B. Lines longer than 80 chars:
>
> I try hard to to keep things within 80 chars with tab stop set to 4 chars.
> Do you see any changes where this is not the case?
Again, this is a fairly minor thing--you don't have to waste too much time on
this. When I look at the patch I still seem some lines that are longer than 80
chars with a 4-char tab, but as you said this can be editor-dependent. So long
as you're aware of the issue and trying to respect it, I'm not going to complain.
Some other things to note:
1 -- SubstituteExpressionVisitor.java:
* The copyright says 1998, 2004; I think this is supposed to just say 2006
since that's when the file was created.
2 -- I see that in version 4 of the patch you updated UnaryOperatorNode to have
the following logic in isEquivalent():
+ UnaryOperatorNode other = (UnaryOperatorNode)o;
+ return (operator.equals(other.operator) &&
+ ((operand == other.operand)||
+ ((operand != null) && operand.isEquivalent(other.operand))));
Did you become aware of this problem because of some test(s) that you were
running? If so, it'd be great if you could add the actual test case(s) to your
JUnit test for regression purposes. The more tests there are to check for these
kinds of subtleties, the better...
And finally:
> Manish Khettry updated DERBY-883:
> ---------------------------------
>
> Attachment: 883.patch4.txt
>
> I found a problem in one of my changes. Also got rid of a lot of
> white space only diffs.
In the future it would be helpful if you could say what changed between versions
of the patch, so that reviewers don't have to scour the patches to try to figure
out what changed. I managed to find out what "a problem" meant in this case,
but it's generally a good idea to explicitly state the problem/changes in the
comments. Makes my life as a reviewer easier :)
Thanks for continuing to improve these patches,
Army
Re: [jira] Updated: (DERBY-883) Enhance GROUP BY clause to support expressions instead of just column references.
Posted by Manish Khettry <ma...@gmail.com>.
On 7/24/06, Army <qo...@gmail.com> wrote:
>
> Hi Manish,
>
> I have to be honest and say that I haven't taken the time to fully
> understand
> the "guts" of this patch--namely, the part where you rewrite the group-by
> tree
> in GroupByNode.addAggregateColumns(). But I did do some review of the
> overall
> patch and I have the following comments. If you can address these I think
> the
> resultant patch will be a lot easier to read (and will also be smaller :)
> so
> that I or anyone else reviewing can focus more on the "guts" of what this
> patch
> is really doing...
>
> ------------------------------------------------------------------
>
> 0) Patch doesn't apply cleanly with latest codeline--there appear to be
> conflicts with the JUnit test changes and also one conflict in
> sqlgrammar.jj. I
> manually resolved these conflicts in my local codeline so that I could run
> some
> tests/view the code, but I think it'd be appropriate to post an updated
> patch
> (sync'd with the latest codeline) to ease the review process for others.
I have updated the bug with a new patch after resolving the conflict.
1) TernarnyOpNode.isEquivalent(): I think we need to check "receiver"
> equivalence as well, in addition to left and right operands. For example,
> I did
> the following and I received an error as expected:
yes, you are right on this one. The new patch fixes this.
2) Aren't we losing information with this change?
>
> In VerifyAggregateExpressionsVisitor.java:
>
> - throw
> StandardException.newException(
> SQLState.LANG_INVALID_COL_REF_GROUPED_SELECT_LIST,
> cr.getSQLColumnName());
> + throw
> StandardException.newException(SQLState.LANG_INVALID_GROUPED_SELECT_LIST);
>
> Before your changes we see the column reference name:
>
> ij> select substr(vc, 3, 4) from s1 group by vc2;
> ERROR 42Y36: Column reference 'VC' is invalid. For a SELECT list with a
> GROUP
> BY, the list may only contain grouping columns and valid aggregate
> expressions.
>
> But after the patch is applied, we lose the name of the invalid column
> reference:
>
> ij> select substr(vc, 3, 4) from s1 group by vc2;
> ERROR 42Y30: The SELECT list of a grouped query contains at least one
> invalid
> expression. If a SELECT list has a GROUP BY, the list may only contain
> valid
> grouping expressions and valid aggregate expressions.
>
> Generally it seems like a good idea to provide the user with as much
> information
> as possible for error conditions like this. Is it possible to preserve
> the
> result column name in cases like this (i.e. cases where we do actually
> know what
> the invalid column reference is)?
Actually the existing error message no longer makes sense with group by
expressions. For example, the line of code that you point to, also catches
errors like this:
select c1+1, avg(c2) from test group by c1+10;
Does the existing error message make sense for this query?
3) In UnaryOperatorNode.java the check for equivalence of the operand has
> three
> parts to it:
>
> + return (operator.equals(other.operator)
> + || (operand == other.operand)
> + || ((operand != null) && operand.isEquivalent
> (other)));
>
> But for other classes--such as BinaryOperatorNode.java--the check is only
> a
> single call to "isEquivalent" for each operand:
>
> + return methodName.equals(other.methodName)
> + && leftOperand.isEquivalent(other.leftOperand)
> + && rightOperand.isEquivalent(other.rightOperand);
>
> Can you explain why the operand for UnaryOperatorNode has three checks
> (.equals(), "==", and .isEquivalent()) while the operands for
> BinaryOperatorNode
> only have a single check (.isEquivalent())? Some comments to describe the
> difference might be useful in these classes.
If you look at bindUnaryOperator for UnaryOperatorNode, you will see that
it is possible for operand to be null; not so in BinaryOperatorNode.
3) I think there are several places in this patch where you have comments
> explaining "what" by not explaining "why"--and I think the latter would be
> helpful. For example:
>
> In VerifyAggregateExpressionsVisitor.java:
>
> Why do we disallow expressions involving native java computation in the
> following code?
>
> + } else if (node instanceof JavaToSQLValueNode)
> + {
> + // disallow any expression which involves native java computation.
> + throw StandardException.newException( (groupByList == null) ?
> + SQLState.LANG_INVALID_NON_GROUPED_SELECT_LIST :
> + SQLState.LANG_INVALID_GROUPED_SELECT_LIST);
It is not possible to compare java expressions for equivalence. I will
update the comments.
Why do we not visit children under subqueries in the following code?
>
> /**
> - * Don't visit children under an aggregate
> + * Don't visit children under an aggregate, subquery or any node
> which
> + * is equivalent to any of the group by expressions.
>
> In SelectNode.java:
There is no code change here-- I just updated the comment to reflect
existing behavior. Honestly, I do't know why we do not visit subqueries.
Why do we need to add a call to preprocess in the following code?
>
> + if (groupByList != null)
> + {
> + groupByList.preprocess(numTables, fromList, whereSubquerys,
> wherePredicates);
> + }
> +
>
> For this one you can probably just use the comments that you already
> included in
> your patch description, namely: "This is needed because expressions can
> get
> rewritten in the select list but not in the grouping list causing problems
> when
> the result set is generated." It'd be great if this explanation was
> included in
> the actual code.
Makes sense. done.
In GroupByNode.java:
>
> Why do we always say that the source is not in sorted order if we have an
> expression instead of a ColumnReference?
>
> - crs[index] = gc.getColumnReference();
> + if (gc.getColumnExpression() instanceof ColumnReference)
> + {
> + crs[index] = (ColumnReference)gc.getColumnExpression();
> + }
> + else
> + {
> + isInSortedOrder = false;
> + break;
> + }
If you look at how isInSortedOrder is used or look at the javadoc for
ResultSet#isOrderedOn, you will see that this optimization applies only for
column references.
Also, in the comment you posted to DERBY-883 with the patch you explained
> how
> the rewrite is different now:
>
> > o the rewrite logic is a bit different now. Earlier, we would change
> > each unaggregate ColumnReference in the select list and point it to the
> > GroupByColumn. Now we replace each group by expression that we find
> > in the projection list with a virtual column node that effectively
> points
> > to a result column in the result set doing the group by. In addition
> > the original routine which did the rewrite is now split up into two
> separate
> > and smaller routines: addUnAggColumns and addAggregateColumns.
>
> It would be great if this explanation could be added to the relevant parts
> in
> the code and/or if the comments for that section could be updated to match
> the
> new behavior. This will make it easier for people to understand what is
> going
> on when they read the code several years from now.
OK, done.
In GroupByList.java:
>
> Why don't we need to add a ResultColumn/ColumnReference pair to
> SelectNode's RCL
> when the grouping column is an expression (as opposed to a
> ColumnReference), as
> in the following code?
>
> @@ -185,7 +174,8 @@
> /* If no match found in the SELECT list, then add a matching
> * ResultColumn/ColumnReference pair to the SelectNode's RCL.
> */
> - if (! matchFound)
> + if (! matchFound &&
> + groupingCol.getColumnExpression() instanceof ColumnReference)
> {
> ResultColumn newRC;
That is a good question-- this bit of code add invisible column references
when a column reference in the group by list, is not in the projection list;
i.e.
select c2, sum(c3) from test group by c1;
will add c1 to the projection list as a generated column. I'm not sure if
you want to do the same kind of rewrite for expressions. I thought this
wasn't worth doing but I'm not sure anymore.
These are just some examples; I think there are others too. If you can look
> again at the patch and try to add comments explaining *why* we do or do
> not need
> to do things like this, that would be a huge help to those reading the
> code now
> and in the future. These don't have to be extensive--even just a line or
> two
> can be very helpful.
>
> 4) In GroupByColumn and GroupByList you have deleted a bunch of methods
> that
> apparently are unused. While this is a useful thing to do, I don't know
> if we
> want to do that as part of the DERBY-883 patch--it makes the diff bigger
> and
> doesn't add to the patch functionally. Instead you could file a new Jira
> entry
> and attach a separate patch that removes the unnecessary methods to that
> Jira
> issue, to be reviewed/committed separately. Or you could create a
> follow-up
> patch for this issue that just separates out the changes for removing
> these methods.
Most if not all of the routines which are deleted were made redundant by
this fix. For example, getColumnReference in GroupByColumn is not only
unused it makes no sense with group by expressions. I think its good to get
rid of this code but if you want, I'd be perfectly willing to add dead code
back in the source tree.
I disagree with you here-- when a contributor has spent a considerable
amount of time understanding and making a lot of changes to a class, it
makes sense to remove dead code at the same time. If you don't, stuff like
this will likely get ignored or slip through the cracks, imho.
5) Extraneous diff in QueryTreeNode.java? Are any of these changes actually
> required or are they just there for debugging? If they are an intentional
> part
> of the patch then a) comments explaining these changes would be helpful,
> and b)
> it might be good to actually delete the commented out lines altogether,
> instead
> of leaving them in to clutter the code.
I've removed this from the new patch.
6) New methods could use javadoc explaining in more detail what they do:
>
> GroupByNode.addUnAggColumns()
> GroupByNode.addAggregateColumns()
OK, done.
7) There are a couple of places where you have commented lines out instead
> of
> deleting them. If there's a reason you left these in, then comments
> explaining
> that would be great; otherwise I think it's cleaner to just delete the
> lines
> altogether. For example:
>
> In GroupByColumn.java:
>
> @@ -48,7 +48,8 @@
> */
> public void init(Object colRef)
> {
> - this.colRef = (ColumnReference) colRef;
> + //this.colRef = (ColumnReference) colRef;
> + this.columnExpression = (ValueNode)colRef;
> }
>
> /**
yes, you're right. I've removed it.
8) I think the diff for master/subquery.out has some stuff from a failed
> merge
> in it:
yes, this is an old diff hanging around; removed it.
> A. Unnecessary white-space changes?
Atleast some of these were done to improve existing indentation. Next time,
I'll resist the urge to improve readability.
> B. Lines longer than 80 chars:
I try hard to to keep things within 80 chars with tab stop set to 4 chars.
Do you see any changes where this is not the case?
Thanks for taking the time to go through these changes. I've attached a new
patch which incorporates most, if not all of your feedback. Other minor
changes from the first patch-- moved the junit test case to a different
package, and added more javadoc comments to the new method in
BaseJDBCTestCase and made the indentation conform to the one in this file
which does not seem to use tabs.
Thanks,
Manish
Re: [jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by Army <qo...@gmail.com>.
Manish Khettry wrote:
> On the point of 80 chars per line [...]
As I mentioned, this was just me being picky. Instead of battling out the tab
question, feel free to leave the lines as they are if they look good in your
editor. This isn't something that should block/hinder your patch, just a little
thing I noticed when reviewing and thought I'd bring up...*shrug*
Army
Re: [jira] Updated: (DERBY-883) Enhance GROUP BY clause to support expressions instead of just column references.
Posted by Manish Khettry <ma...@yahoo.com>.
Army,
Thanks for the feeback. I appreciate it. You've
identified atleast one (perhaps 2) bugs with my code
changes.
On the point of 80 chars per line, I can't help but
ask: does a tab count as 4 chars or 8?
The conventional wisdom from an earlier discussion was
that you set your editor to treat a tab as 4 chars
which I did and the offending lines that you point to
were well within the 80 char margin on my editor.
m
--- Army <qo...@gmail.com> wrote:
> Hi Manish,
>
> I have to be honest and say that I haven't taken the
> time to fully understand
> the "guts" of this patch--namely, the part where you
> rewrite the group-by tree
> in GroupByNode.addAggregateColumns(). But I did do
> some review of the overall
> patch and I have the following comments. If you can
> address these I think the
> resultant patch will be a lot easier to read (and
> will also be smaller :) so
> that I or anyone else reviewing can focus more on
> the "guts" of what this patch
> is really doing...
>
>
------------------------------------------------------------------
>
> 0) Patch doesn't apply cleanly with latest
> codeline--there appear to be
> conflicts with the JUnit test changes and also one
> conflict in sqlgrammar.jj. I
> manually resolved these conflicts in my local
> codeline so that I could run some
> tests/view the code, but I think it'd be appropriate
> to post an updated patch
> (sync'd with the latest codeline) to ease the review
> process for others.
>
> 1) TernarnyOpNode.isEquivalent(): I think we need to
> check "receiver"
> equivalence as well, in addition to left and right
> operands. For example, I did
> the following and I received an error as expected:
>
> ij> create table s1 (vc varchar(20), vc2
> varchar(10));
> 0 rows inserted/updated/deleted
>
> ij> insert into s1 values ('allo', 'bye'), ('inigo',
> 'montoya'), ('six','fingers');
> 3 rows inserted/updated/deleted
>
> ij> select substr(vc, 3, 4) from s1 group by vc2;
> ERROR 42Y30: The SELECT list of a grouped query
> contains at least one invalid
> expression. If a SELECT list has a GROUP BY, the
> list may only contain valid
> grouping expressions and valid aggregate
> expressions.
>
> I then I did the following, expecting to see the
> same error--but this actually
> works:
>
> ij> select substr(vc, 3, 4) from s1 group by
> substr(vc2, 3, 4);
> 1
> ----
> igo
> lo
> x
>
> 3 rows selected
>
> I think this is because
> TernaryOperatorNode.isEquivalent() doesn't compare
> "receiver" and thus thinks that vc and vc2 are the
> same, hence allowing the
> group by. Is this supposed to work or is it
> supposed to throw an error? My
> understanding is that it should fail, but I could of
> course be wrong...
>
> 2) Aren't we losing information with this change?
>
> In VerifyAggregateExpressionsVisitor.java:
>
> - throw
>
StandardException.newException(SQLState.LANG_INVALID_COL_REF_GROUPED_SELECT_LIST,
>
> cr.getSQLColumnName());
> + throw
>
StandardException.newException(SQLState.LANG_INVALID_GROUPED_SELECT_LIST);
>
> Before your changes we see the column reference
> name:
>
> ij> select substr(vc, 3, 4) from s1 group by vc2;
> ERROR 42Y36: Column reference 'VC' is invalid. For
> a SELECT list with a GROUP
> BY, the list may only contain grouping columns and
> valid aggregate expressions.
>
> But after the patch is applied, we lose the name of
> the invalid column reference:
>
> ij> select substr(vc, 3, 4) from s1 group by vc2;
> ERROR 42Y30: The SELECT list of a grouped query
> contains at least one invalid
> expression. If a SELECT list has a GROUP BY, the
> list may only contain valid
> grouping expressions and valid aggregate
> expressions.
>
> Generally it seems like a good idea to provide the
> user with as much information
> as possible for error conditions like this. Is it
> possible to preserve the
> result column name in cases like this (i.e. cases
> where we do actually know what
> the invalid column reference is)?
>
> 3) In UnaryOperatorNode.java the check for
> equivalence of the operand has three
> parts to it:
>
> + return (operator.equals(other.operator)
> + || (operand == other.operand)
> + || ((operand != null) &&
> operand.isEquivalent(other)));
>
> But for other classes--such as
> BinaryOperatorNode.java--the check is only a
> single call to "isEquivalent" for each operand:
>
> + return methodName.equals(other.methodName)
> + &&
> leftOperand.isEquivalent(other.leftOperand)
> + &&
> rightOperand.isEquivalent(other.rightOperand);
>
> Can you explain why the operand for
> UnaryOperatorNode has three checks
> (.equals(), "==", and .isEquivalent()) while the
> operands for BinaryOperatorNode
> only have a single check (.isEquivalent())? Some
> comments to describe the
> difference might be useful in these classes.
>
> 3) I think there are several places in this patch
> where you have comments
> explaining "what" by not explaining "why"--and I
> think the latter would be
> helpful. For example:
>
> In VerifyAggregateExpressionsVisitor.java:
>
> Why do we disallow expressions involving native java
> computation in the
> following code?
>
> + } else if (node instanceof JavaToSQLValueNode)
> + {
> + // disallow any expression which involves
> native java computation.
> + throw StandardException.newException(
> (groupByList == null) ?
> +
> SQLState.LANG_INVALID_NON_GROUPED_SELECT_LIST :
> +
> SQLState.LANG_INVALID_GROUPED_SELECT_LIST);
>
> Why do we not visit children under subqueries in the
> following code?
>
> /**
> - * Don't visit children under an aggregate
> + * Don't visit children under an aggregate,
> subquery or any node which
> + * is equivalent to any of the group by
> expressions.
>
> In SelectNode.java:
>
> Why do we need to add a call to preprocess in the
> following code?
>
> + if (groupByList != null)
> + {
> + groupByList.preprocess(numTables, fromList,
> whereSubquerys, wherePredicates);
> + }
> +
>
> For this one you can probably just use the comments
> that you already included in
> your patch description, namely: "This is needed
> because expressions can get
> rewritten in the select list but not in the grouping
> list causing problems when
> the result set is generated." It'd be great if this
> explanation was included in
> the actual code.
>
> In GroupByNode.java:
>
> Why do we always say that the source is not in
> sorted order if we have an
> expression instead of a ColumnReference?
>
>
=== message truncated ===
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
Re: [jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by Army <qo...@gmail.com>.
Hi Manish,
I have to be honest and say that I haven't taken the time to fully understand
the "guts" of this patch--namely, the part where you rewrite the group-by tree
in GroupByNode.addAggregateColumns(). But I did do some review of the overall
patch and I have the following comments. If you can address these I think the
resultant patch will be a lot easier to read (and will also be smaller :) so
that I or anyone else reviewing can focus more on the "guts" of what this patch
is really doing...
------------------------------------------------------------------
0) Patch doesn't apply cleanly with latest codeline--there appear to be
conflicts with the JUnit test changes and also one conflict in sqlgrammar.jj. I
manually resolved these conflicts in my local codeline so that I could run some
tests/view the code, but I think it'd be appropriate to post an updated patch
(sync'd with the latest codeline) to ease the review process for others.
1) TernarnyOpNode.isEquivalent(): I think we need to check "receiver"
equivalence as well, in addition to left and right operands. For example, I did
the following and I received an error as expected:
ij> create table s1 (vc varchar(20), vc2 varchar(10));
0 rows inserted/updated/deleted
ij> insert into s1 values ('allo', 'bye'), ('inigo', 'montoya'), ('six','fingers');
3 rows inserted/updated/deleted
ij> select substr(vc, 3, 4) from s1 group by vc2;
ERROR 42Y30: The SELECT list of a grouped query contains at least one invalid
expression. If a SELECT list has a GROUP BY, the list may only contain valid
grouping expressions and valid aggregate expressions.
I then I did the following, expecting to see the same error--but this actually
works:
ij> select substr(vc, 3, 4) from s1 group by substr(vc2, 3, 4);
1
----
igo
lo
x
3 rows selected
I think this is because TernaryOperatorNode.isEquivalent() doesn't compare
"receiver" and thus thinks that vc and vc2 are the same, hence allowing the
group by. Is this supposed to work or is it supposed to throw an error? My
understanding is that it should fail, but I could of course be wrong...
2) Aren't we losing information with this change?
In VerifyAggregateExpressionsVisitor.java:
- throw
StandardException.newException(SQLState.LANG_INVALID_COL_REF_GROUPED_SELECT_LIST,
cr.getSQLColumnName());
+ throw
StandardException.newException(SQLState.LANG_INVALID_GROUPED_SELECT_LIST);
Before your changes we see the column reference name:
ij> select substr(vc, 3, 4) from s1 group by vc2;
ERROR 42Y36: Column reference 'VC' is invalid. For a SELECT list with a GROUP
BY, the list may only contain grouping columns and valid aggregate expressions.
But after the patch is applied, we lose the name of the invalid column reference:
ij> select substr(vc, 3, 4) from s1 group by vc2;
ERROR 42Y30: The SELECT list of a grouped query contains at least one invalid
expression. If a SELECT list has a GROUP BY, the list may only contain valid
grouping expressions and valid aggregate expressions.
Generally it seems like a good idea to provide the user with as much information
as possible for error conditions like this. Is it possible to preserve the
result column name in cases like this (i.e. cases where we do actually know what
the invalid column reference is)?
3) In UnaryOperatorNode.java the check for equivalence of the operand has three
parts to it:
+ return (operator.equals(other.operator)
+ || (operand == other.operand)
+ || ((operand != null) && operand.isEquivalent(other)));
But for other classes--such as BinaryOperatorNode.java--the check is only a
single call to "isEquivalent" for each operand:
+ return methodName.equals(other.methodName)
+ && leftOperand.isEquivalent(other.leftOperand)
+ && rightOperand.isEquivalent(other.rightOperand);
Can you explain why the operand for UnaryOperatorNode has three checks
(.equals(), "==", and .isEquivalent()) while the operands for BinaryOperatorNode
only have a single check (.isEquivalent())? Some comments to describe the
difference might be useful in these classes.
3) I think there are several places in this patch where you have comments
explaining "what" by not explaining "why"--and I think the latter would be
helpful. For example:
In VerifyAggregateExpressionsVisitor.java:
Why do we disallow expressions involving native java computation in the
following code?
+ } else if (node instanceof JavaToSQLValueNode)
+ {
+ // disallow any expression which involves native java computation.
+ throw StandardException.newException( (groupByList == null) ?
+ SQLState.LANG_INVALID_NON_GROUPED_SELECT_LIST :
+ SQLState.LANG_INVALID_GROUPED_SELECT_LIST);
Why do we not visit children under subqueries in the following code?
/**
- * Don't visit children under an aggregate
+ * Don't visit children under an aggregate, subquery or any node which
+ * is equivalent to any of the group by expressions.
In SelectNode.java:
Why do we need to add a call to preprocess in the following code?
+ if (groupByList != null)
+ {
+ groupByList.preprocess(numTables, fromList, whereSubquerys, wherePredicates);
+ }
+
For this one you can probably just use the comments that you already included in
your patch description, namely: "This is needed because expressions can get
rewritten in the select list but not in the grouping list causing problems when
the result set is generated." It'd be great if this explanation was included in
the actual code.
In GroupByNode.java:
Why do we always say that the source is not in sorted order if we have an
expression instead of a ColumnReference?
- crs[index] = gc.getColumnReference();
+ if (gc.getColumnExpression() instanceof ColumnReference)
+ {
+ crs[index] = (ColumnReference)gc.getColumnExpression();
+ }
+ else
+ {
+ isInSortedOrder = false;
+ break;
+ }
Also, in the comment you posted to DERBY-883 with the patch you explained how
the rewrite is different now:
> o the rewrite logic is a bit different now. Earlier, we would change
> each unaggregate ColumnReference in the select list and point it to the
> GroupByColumn. Now we replace each group by expression that we find
> in the projection list with a virtual column node that effectively points
> to a result column in the result set doing the group by. In addition
> the original routine which did the rewrite is now split up into two separate
> and smaller routines: addUnAggColumns and addAggregateColumns.
It would be great if this explanation could be added to the relevant parts in
the code and/or if the comments for that section could be updated to match the
new behavior. This will make it easier for people to understand what is going
on when they read the code several years from now.
In GroupByList.java:
Why don't we need to add a ResultColumn/ColumnReference pair to SelectNode's RCL
when the grouping column is an expression (as opposed to a ColumnReference), as
in the following code?
@@ -185,7 +174,8 @@
/* If no match found in the SELECT list, then add a matching
* ResultColumn/ColumnReference pair to the SelectNode's RCL.
*/
- if (! matchFound)
+ if (! matchFound &&
+ groupingCol.getColumnExpression() instanceof ColumnReference)
{
ResultColumn newRC;
These are just some examples; I think there are others too. If you can look
again at the patch and try to add comments explaining *why* we do or do not need
to do things like this, that would be a huge help to those reading the code now
and in the future. These don't have to be extensive--even just a line or two
can be very helpful.
4) In GroupByColumn and GroupByList you have deleted a bunch of methods that
apparently are unused. While this is a useful thing to do, I don't know if we
want to do that as part of the DERBY-883 patch--it makes the diff bigger and
doesn't add to the patch functionally. Instead you could file a new Jira entry
and attach a separate patch that removes the unnecessary methods to that Jira
issue, to be reviewed/committed separately. Or you could create a follow-up
patch for this issue that just separates out the changes for removing these methods.
5) Extraneous diff in QueryTreeNode.java? Are any of these changes actually
required or are they just there for debugging? If they are an intentional part
of the patch then a) comments explaining these changes would be helpful, and b)
it might be good to actually delete the commented out lines altogether, instead
of leaving them in to clutter the code.
6) New methods could use javadoc explaining in more detail what they do:
GroupByNode.addUnAggColumns()
GroupByNode.addAggregateColumns()
7) There are a couple of places where you have commented lines out instead of
deleting them. If there's a reason you left these in, then comments explaining
that would be great; otherwise I think it's cleaner to just delete the lines
altogether. For example:
In GroupByColumn.java:
@@ -48,7 +48,8 @@
*/
public void init(Object colRef)
{
- this.colRef = (ColumnReference) colRef;
+ //this.colRef = (ColumnReference) colRef;
+ this.columnExpression = (ValueNode)colRef;
}
/**
8) I think the diff for master/subquery.out has some stuff from a failed merge
in it:
Index: java/testing/org/apache/derbyTesting/functionTests/master/subquery.out
===================================================================
--- java/testing/org/apache/derbyTesting/functionTests/master/subquery.out
(revision 422486)
+++ java/testing/org/apache/derbyTesting/functionTests/master/subquery.out
(working copy)
@@ -1016,8 +1016,14 @@
Number of rows visited=9
Scan type=heap
start position:
+<<<<<<< .mine
null stop position:
+null
+ scan qualifiers:
+=======
+null stop position:
null scan qualifiers:
+>>>>>>> .r422486
None
next qualifiers:
None
**** Nit-picking: Changes that you don't *have* to do but that might help in
cleaning up the patch.
A. Unnecessary white-space changes?
In SelectNode.java:
- VerifyAggregateExpressionsVisitor visitor =
- new VerifyAggregateExpressionsVisitor(groupByList);
+ VerifyAggregateExpressionsVisitor visitor =
+ new VerifyAggregateExpressionsVisitor(groupByList);
resultColumns.accept(visitor);
- }
-
+ }
+
+
+
In GroupByNode.java:
- C_NodeTypes.RESULT_COLUMN_LIST,
- getContextManager()),
- ((FromTable) childResult).getTableNumber());
+ C_NodeTypes.RESULT_COLUMN_LIST,
+ getContextManager()),
+ ((FromTable) childResult).getTableNumber());
...
- ** For each aggregate
- */
+ ** For each aggregate
+ */
...
-
+
/*
- ** AGG RESULT: Set the aggregate result to null in the
- ** bottom project restrict.
- */
+ ** AGG RESULT: Set the aggregate result to null in the
+ ** bottom project restrict.
+ */
... etc.
There are quite a few white-space changes in this patch that make it hard to
figure out what has actually changed.
B. Lines longer than 80 chars:
In SelectNode.java:
@@ -965,6 +956,11 @@
wherePredicates);
}
+ if (groupByList != null)
+ {
+ groupByList.preprocess(numTables, fromList, whereSubquerys, wherePredicates);
+ }
+
In VerifyAggregateExpressionsVisitor.java:
+ return ((node instanceof AggregateNode) ||
+ (node instanceof SubqueryNode) ||
+ (node instanceof ValueNode &&
+ groupByList != null
+ && groupByList.findGroupingColumn((ValueNode)node) != null));
I know, this whole 80 character thing is a bit picky--but that seems to be the
rule adopted by most developers in the Derby code, so I'm just bringing it up...
------------------------------------------------------------------
I know that's a lot of feedback and very little of it has to do with the actual
correctness of the code--so my apologies if this is a bit annoying. But I do
think that some additional comments and an effort to reduce the extra diffs will
make this a cleaner patch and will help developers in the long and short-term
who might find themselves reading this code.
Functionally, this is a great patch--an excellent feature enhancement for Derby.
Thank you so much for taking the time to figure out all of the details--and
for writing such extensive tests, as well. I hope this feedback doesn't
discourage you from continuing to work on this or other Derby changes: I'm just
posting feedback based on my own opinions. If you disagree with anything I've
written here, please feel free to speak up!
Thanks again for all of the work on this,
Army
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Manish Khettry updated DERBY-883:
---------------------------------
Derby Info: [Patch Available]
Functionally, this patch supports expressions in the group by list. An expression
in the group by list can also be used in the select list. For example if the grouping
expression is c1+c2, then a valid expression in the select list would be
c1+c2+1. The expression matching facility is not smart enough to allow c1+1+c2.
This patch does not allow the use of grouping expressions in the having clause
or order by clause. These are restrictions for now which should be removed
eventually. For more information on group by expressions:
http://www.pdc.kth.se/doc/SP/manuals/db2-5.0/html/db2s0/db2s0176.htm#HDRGRPBY
Another change has been to allow the use of duplicate grouping expressions: i.e
select a, sum(b) from test group by a,a;
Error messages have been changed. The most noticeable change is the use
of the more general sql exception: LANG_INVALID_GROUPED_SELECT_LIST.
VerifyAggregateExpressionsVisitor
o disallows java nodes. This should take care of functions in java.
o the skipChildren method doesn't traverse a subtree that contains
any grouping expression.
o the error messages are more appropriate-- LANG_INVALID_GROUPED_SELECT_LIST.
SelectNode
o calls preprocess on the group by list. This is needed because expressions
can get rewritten in the select list but not in the grouping list causing
problems when the result set is generated.
GroupByNode
o changes to init() take care of the case where gropuing expressions is
not a column reference.
o the rewrite logic is a bit different now. Earlier, we would change
each unaggregate ColumnReference in the select list and point it to the
GroupByColumn. Now we replace each group by expression that we find
in the projection list with a virtual column node that effectively points
to a result column in the result set doing the group by. In addition
the original routine which did the rewrite is now split up into two separate
and smaller routines: addUnAggColumns and addAggregateColumns.
GroupByColumn
o now uses a ValueNode instead of a ColumnReference. Unused methods zapped.
GroupByDiff
o verifyUniqueGroupingColumn discarded. This was a restriction earlier and
it does not make sense.
o findGroupingColumn now does the hard work of finding a group by column
for a given expression.
sqlgrammer
o use additiveExpression instead of columnReference.
GroupByExpressionTest.java
o JUnit test case for this functionality.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Manish Khettry updated DERBY-883:
---------------------------------
Attachment: 883.patch7.txt
Synced to rev 434408. Use the new JUnit test case/test setup classes. Passed derbylang.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Daniel John Debrunner updated DERBY-883:
----------------------------------------
Fix Version/s: 10.3.0.0
(was: 10.2.1.0)
Change was commited to trunk (10.3)
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.3.0.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Manish Khettry updated DERBY-883:
---------------------------------
Attachment: 883.patch6.txt
svn update to revision 430199. Hopefully this patch will apply cleanly.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Mike Matrigali (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Mike Matrigali updated DERBY-883:
---------------------------------
Affects Version/s: 10.2.1.0
This looks like a good thing to get into 10.2,.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Mike Matrigali (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=all ]
Mike Matrigali updated DERBY-883:
---------------------------------
Fix Version/s: 10.2.1.0
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (DERBY-883) Enhance GROUP BY clause to support
expressions instead of just column references.
Posted by "Manish Khettry (JIRA)" <de...@db.apache.org>.
[ http://issues.apache.org/jira/browse/DERBY-883?page=comments#action_12430672 ]
Manish Khettry commented on DERBY-883:
--------------------------------------
I'm not sure what you mean by function changes-- one new function has been added to BaseJDBCTestCase (although it was added to the "old" BaseJDBCTestCase as well). The suite method in the GroupByExpressionTest.junit as well as the setUp/tearDown methods have changed slightly, but no new functions anywhere and no methods modified anywhere else.
> Enhance GROUP BY clause to support expressions instead of just column references.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-883
> URL: http://issues.apache.org/jira/browse/DERBY-883
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.1.2.1, 10.2.1.0
> Environment: JDK 1.5.0_05
> Reporter: Lluis Turro
> Assigned To: Manish Khettry
> Fix For: 10.2.1.0
>
> Attachments: 883.patch.txt, 883.patch3.txt, 883.patch4.txt, 883.patch5.txt, 883.patch6.txt, 883.patch6.txt, 883.patch7.txt
>
>
> This select would return an error syntax on finding "(" after month if group by clause:
> select idissue, month(creation), year(creation), count(distinct idissue)
> where
> ....
> group by idissue, month(creation), year(creation)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira