Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expressions not supported in the GROUP BY or ORDER BY clause. #3193

Closed
monetdb-team opened this issue Nov 30, 2020 · 0 comments
Closed

Expressions not supported in the GROUP BY or ORDER BY clause. #3193

monetdb-team opened this issue Nov 30, 2020 · 0 comments
Assignees

Comments

@monetdb-team
Copy link

@monetdb-team monetdb-team commented Nov 30, 2020

Date: 2012-11-24 05:29:07 +0100
From: Brandon Jackson <>
To: @njnes
Version: 11.13.5 (Oct2012-SP1)
CC: bugs-sql, monetdbuser, @njnes

Last updated: 2013-02-19 13:17:59 +0100

Comment 17990

Date: 2012-11-24 05:29:07 +0100
From: Brandon Jackson <>

User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0
Build Identifier:

Pasted email response from Fabian below:

Hi Brandon,

On 19-11-2012 16:24:09 -0600, Brandon Jackson wrote:

// original query

// expressions are not supported in group by and order by

select "time_by_day"."the_year" as "c0", "the_year" || '-12-31' as
"c1" from "time_by_day" as "time_by_day" group by "time_by_day"."the_year", "the_year"
|| '-12-31' order by CASE WHEN "time_by_day"."the_year" IS NULL THEN
|| 1 ELSE
0 END, "time_by_day"."the_year" ASC

// this doesn't work because the group by and order by for c0 are
different

select "time_by_day"."the_year" as "c0", "the_year" || '-12-31' as
"c1" from "time_by_day" as "time_by_day" group by c0, c1 order by
CASE WHEN "time_by_day"."the_year" IS NULL THEN 1 ELSE 0 END, "time_by_day"."the_year"
ASC

// this works again

select "time_by_day"."the_year" as "c0", "the_year" || '-12-31' as
"c1" from "time_by_day" as "time_by_day" group by c0, c1 order by
CASE WHEN c0 IS NULL THEN 1 ELSE 0 END, c0 ASC

Use SQL queries to illustrate the problem Mondrian is having:

Query 1, is what we have right now with the dialect just created. If
we turn on the “group by alias” code in Mondrian, I get query 2, but
what we actually need is query 3.

We have looked at the SQL standard as best we can, but cannot find
where it comes right out and says what the outcome should be.
However, from experience we know most databases we’ve encountered
support this. What is your take on the matter?

My feeling is that since group columns need to appear in select, it makes sense that group by doesn't support expressions. The alias solves that, which gets you sort of duplicate elimination for free. I understand you can live with that fine too.

What you end up with is the "mismatch" of c0 and its unmodified original column. I think it's worth filing a bug for this one. Niels can judge whether this can be easily detected by some more checking, or not.

Fabian

--
dr. Fabian Groffen fabian@monetdb.com
column-store pioneer www.monetdb.com

Reproducible: Always

Comment 18020

Date: 2012-11-26 21:52:31 +0100
From: @njnes

are there other examples of the case 2 vs 3. This example maybe fixable but I'm afraid it may require a more general (even larger) patch.

Comment 18083

Date: 2012-11-27 14:35:10 +0100
From: @njnes

Changeset 89a9ebc92eda made by Niels Nes niels@cwi.nl in the MonetDB repo, refers to this bug.

For complete details, see http//devmonetdborg/hg/MonetDB?cmd=changeset;node=89a9ebc92eda

Changeset description:

added tests for Bugs 3193, 3179 and 3172

Fixed bug #3191, we now allow the order by column expressions to refer to
the lower level projection. This is very limited support voor order by with
expressions. Aliases is still preferred.

Fixed bug #3179 (or feature request). We now rewrite batstr.*like + subselect into
*likeselect. Also some cleanup of pushselect optimizer and mergetable related
helper functions in opt_support.c

Fixed bug #3172. We still don't allow multi row input in table functions. But
we now don't crash on it anymore. Still work on mal.multiplex needed (requires
multiple outputs)!

Comment 18513

Date: 2013-02-19 13:17:59 +0100
From: @sjoerdmullender

Feb2013 has been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants