[MONDRIAN-2451] Mondrian generates ORDER BY sql that is invalid with default settings of MySQL 5.7.5+ #669

mr9d · 2016-03-03T13:34:50Z

This is the second attempt to resolve MONDRIAN-2451. Previous pull request was rejected: #661

Accordint to Matt Campbell's suggestion, I've added requiresOrderByAlias()=true for the MySQL dialect. This caused a side effect with "ORDER BY ISNULL(1) ASC, 1 ASC, ISNULL(2) ASC, 2 ASC, etc" statements in SqlTupleReader.java so it was needed to change them to "ORDER BY ISNULL('c0') ASC, 'c0' ASC, ISNULL('c1') ASC, 'c1' ASC, etc".

Still not sure about this change, because of Luc Boudreau's comment in source code:

Sort the union of the cubes.
The order by columns need to be numbers,
not column name strings or expressions.

(SqlTupleReader.java:838)

Anyway, all tests are successful now, so it might be the an answer to MONDRIAN-2451.

mr9d · 2016-03-03T13:36:01Z

@mkambol @lucboudreau please review.

mkambol · 2016-03-03T14:28:18Z

src/main/mondrian/rolap/SqlTupleReader.java

@@ -841,7 +841,7 @@ private void savePartialResult(List<List<RolapMember>> partialResult) {
                if (fullyJoiningBaseCubes.size() > 1) {
                    for (int i = 0; i < types.size(); i++) {
                        unionQuery.addOrderBy(
-                            i + 1 + "",
+                            "c" + i,
                            true,


We probably want to make this change conditional on whether the dialect has requiresOrderByAlias=true. It's possible (likely) that some databases which don't require aliases will fail with the new UNION structure if we don't have a numbered order by list.

And to confirm-- you've tried older versions of MySQL with this change to the UNION sql? (and full test suite?)

Yep, I've tried full test suite locally on MySQL 5.7.10 and on MySQL 5.5 on CI-server.

mkambol · 2016-03-03T14:30:01Z

@lucboudreau, do you remember the background of your comment the "order by columns need to be numbers"? Are there specific databases where this is true?

mkambol · 2016-03-03T14:37:11Z

@mr9d I think we need some new tests that explicitly cover the scenario we're fixing. testLevelKeyAsSqlExpWithAgg fails with the ORDER BY containing an expression, but that was sort of an accidental catch.

It would be good to have more test cases that uses a sql expression in a Level definition, and then explicitly hit the various ways an ORDER BY could occur-- including with a virtual cube.

lucboudreau · 2016-03-03T14:59:05Z

@mkambol Some DBs (Oracle?) need the parent query to order by column ordinals rather than their alias.

mkambol · 2016-03-03T16:42:06Z

Thanks @lucboudreau, that's what I was guessing.

@mr9d - Looking at this again, why is it we need the UNION'd order by to use aliases? Using the ordinal should still work just fine. The reason those tests were failing after the dialect change was because we started wrapping the ordinal in quotes. If we just make sure the UNION'd order by SQL is still valid then it seems like it should be fine to continue using the ordinal.

mr9d · 2016-03-04T08:28:43Z

@mkambol I've had the same idea, but was unable to test it on CI because of server unavailability. Now I see that it might be an another solution.

Please review the latest changes: I've reverted SqlTupleReader back to ordinals usage for UNION queries and updated SqlQuery to make sure that addOrderBy() method will use aliases only if alias is not null and it will be possible to use expressions even if requiresOrderByAlias() is true.

mkambol · 2016-03-04T15:02:13Z

This is looking better, @mr9d. Can you do two things?

Add test cases that use a sql expression in a Level definition, and then explicitly hit the various ways an ORDER BY could occur-- including with a virtual cube.
Add a comment to the .requiresOrderByAlias method in MySqlDialect that references the jira case and describes the scenario where an alias is actually required-- i.e. ISNULL wrapping a SQL expression in an ORDER BY can fail. If I were reviewing the dialect without knowing that bit of trivia I would think the method incorrect.

mkambol · 2016-06-21T14:31:55Z

Also, please squash all these commits.

mr9d · 2016-06-23T08:59:10Z

@mkambol please review. Also I've sent some details via e-mail.

…default settings of MySQL 5.7.5+ - requiresOrderByAlias=true for MySqlDialect. - Conditional change on whether the dialect has requiresOrderByAlias=true. - Using aliases in ORDER BY only for non-null values. - UTs fixes. - MySqlDialect.requiresOrderByAlias() Javadoc is added. - Unit tests for MySQL are added. - UTs fixes.

wingman-pentaho · 2016-06-28T05:47:28Z

Build Failed

❌ Something went wrong while validating this pull request.

Stdout log

(last 100 lines)

Config enriched: If no api token specified, use environment variable
Config enriched: Set the slackbot url
Config enriched: If github SourceControlType and null StatusUpdateType, use github
Config enriched: If github SourceControlType and StatusUpdaterType and null Organization, use the one from SourceRetriever
Config enriched: If github SourceControlType and StatusUpdaterType and null Repository, use the one from SourceRetriever
Config enriched: If github SourceControlType and StatusUpdaterType and null PullRequest, use the one from SourceRetriever
Config enriched: CheckstyleAnalyzer is enabled by default
Retrieving source with org.pentaho.build.buddy.bundles.source.github.SlackbotGithubSourceRetriever@24566d1b

Stderr log

(last 100 lines)

Cloning into 'mondrian-base-669-43226b1f-90d3-4131-959b-b76960cbbeda'...
java.lang.Exception: java.io.IOException: java.io.IOException: Exit code: 128 for command: [git, merge, --no-edit, --no-ff, pullRequest]
    at org.pentaho.build.buddy.bundles.orchestrator.OrchestratorImpl.orchestrate(OrchestratorImpl.java:338)
    at Proxy6d9c4c1e_c078_4506_8f78_6b5f251d94f4.orchestrate(Unknown Source)
    at org.pentaho.build.buddy.bundles.rest.OrchestratorRestService$1.write(OrchestratorRestService.java:62)
    at org.apache.cxf.jaxrs.provider.BinaryDataProvider.writeTo(BinaryDataProvider.java:172)
    at org.apache.cxf.jaxrs.utils.JAXRSUtils.writeMessageBody(JAXRSUtils.java:1381)
    at org.apache.cxf.jaxrs.interceptor.JAXRSOutInterceptor.serializeMessage(JAXRSOutInterceptor.java:244)
    at org.apache.cxf.jaxrs.interceptor.JAXRSOutInterceptor.processResponse(JAXRSOutInterceptor.java:120)
    at org.apache.cxf.jaxrs.interceptor.JAXRSOutInterceptor.handleMessage(JAXRSOutInterceptor.java:83)
    at org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:308)
    at org.apache.cxf.interceptor.OutgoingChainInterceptor.handleMessage(OutgoingChainInterceptor.java:83)
    at org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:308)
    at org.apache.cxf.transport.ChainInitiationObserver.onMessage(ChainInitiationObserver.java:121)
    at org.apache.cxf.transport.http.AbstractHTTPDestination.invoke(AbstractHTTPDestination.java:253)
    at org.apache.cxf.transport.servlet.ServletController.invokeDestination(ServletController.java:234)
    at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:208)
    at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:160)
    at org.apache.cxf.transport.servlet.CXFNonSpringServlet.invoke(CXFNonSpringServlet.java:180)
    at org.apache.cxf.transport.servlet.AbstractHTTPServlet.handleRequest(AbstractHTTPServlet.java:298)
    at org.apache.cxf.transport.servlet.AbstractHTTPServlet.doPost(AbstractHTTPServlet.java:217)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
    at org.apache.cxf.transport.servlet.AbstractHTTPServlet.service(AbstractHTTPServlet.java:273)
    at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:812)
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:587)
    at org.ops4j.pax.web.service.jetty.internal.HttpServiceServletHandler.doHandle(HttpServiceServletHandler.java:70)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
    at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:577)
    at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:223)
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1127)
    at org.ops4j.pax.web.service.jetty.internal.HttpServiceContext.doHandle(HttpServiceContext.java:271)
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:515)
    at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1061)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
    at org.ops4j.pax.web.service.jetty.internal.JettyServerHandlerCollection.handle(JettyServerHandlerCollection.java:80)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
    at org.eclipse.jetty.server.Server.handle(Server.java:499)
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:311)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257)
    at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:544)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555)
    at java.lang.Thread.run(Thread.java:745)
Caused by: java.io.IOException: java.io.IOException: Exit code: 128 for command: [git, merge, --no-edit, --no-ff, pullRequest]
    at org.pentaho.build.buddy.bundles.source.github.GithubSourceRetriever.retrieveSource(GithubSourceRetriever.java:117)
    at Proxyc906d380_6153_4886_b1f7_09cfc0aa32c8.retrieveSource(Unknown Source)
    at org.pentaho.build.buddy.bundles.orchestrator.OrchestratorImpl.orchestrate(OrchestratorImpl.java:295)
    ... 41 more
Caused by: java.io.IOException: Exit code: 128 for command: [git, merge, --no-edit, --no-ff, pullRequest]
    at org.pentaho.build.buddy.util.shell.ShellUtil.executeAndCheck(ShellUtil.java:59)
    at org.pentaho.build.buddy.bundles.source.github.GithubSourceRetriever.retrieveSource(GithubSourceRetriever.java:103)
    ... 43 more

mr9d · 2016-06-28T07:28:59Z

@mkambol please review.

mkambol · 2016-06-28T17:41:07Z

@mr9d We're getting closer. Try running the test suite with aggregate tables turned on. There's a few failures.

mondrian.rolap.aggregates.Use=true
mondrian.rolap.aggregates.Read=true

…default settings of MySQL 5.7.5+ - UTs fixes for mondrian.rolap.aggregates.Use=true and mondrian.rolap.aggregates.Read=true

wingman-pentaho · 2016-06-29T13:40:20Z

Build Failed

❌ Something went wrong while validating this pull request.

Stdout log

(last 100 lines)

Config enriched: If no api token specified, use environment variable
Config enriched: Set the slackbot url
Config enriched: If github SourceControlType and null StatusUpdateType, use github
Config enriched: If github SourceControlType and StatusUpdaterType and null Organization, use the one from SourceRetriever
Config enriched: If github SourceControlType and StatusUpdaterType and null Repository, use the one from SourceRetriever
Config enriched: If github SourceControlType and StatusUpdaterType and null PullRequest, use the one from SourceRetriever
Config enriched: CheckstyleAnalyzer is enabled by default
Retrieving source with org.pentaho.build.buddy.bundles.source.github.SlackbotGithubSourceRetriever@24566d1b

Stderr log

(last 100 lines)

Cloning into 'mondrian-base-669-baf5ca45-df25-4657-bb16-e730fc1e1372'...
java.lang.Exception: java.io.IOException: java.io.IOException: Exit code: 128 for command: [git, merge, --no-edit, --no-ff, pullRequest]
    at org.pentaho.build.buddy.bundles.orchestrator.OrchestratorImpl.orchestrate(OrchestratorImpl.java:338)
    at Proxy6d9c4c1e_c078_4506_8f78_6b5f251d94f4.orchestrate(Unknown Source)
    at org.pentaho.build.buddy.bundles.rest.OrchestratorRestService$1.write(OrchestratorRestService.java:62)
    at org.apache.cxf.jaxrs.provider.BinaryDataProvider.writeTo(BinaryDataProvider.java:172)
    at org.apache.cxf.jaxrs.utils.JAXRSUtils.writeMessageBody(JAXRSUtils.java:1381)
    at org.apache.cxf.jaxrs.interceptor.JAXRSOutInterceptor.serializeMessage(JAXRSOutInterceptor.java:244)
    at org.apache.cxf.jaxrs.interceptor.JAXRSOutInterceptor.processResponse(JAXRSOutInterceptor.java:120)
    at org.apache.cxf.jaxrs.interceptor.JAXRSOutInterceptor.handleMessage(JAXRSOutInterceptor.java:83)
    at org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:308)
    at org.apache.cxf.interceptor.OutgoingChainInterceptor.handleMessage(OutgoingChainInterceptor.java:83)
    at org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:308)
    at org.apache.cxf.transport.ChainInitiationObserver.onMessage(ChainInitiationObserver.java:121)
    at org.apache.cxf.transport.http.AbstractHTTPDestination.invoke(AbstractHTTPDestination.java:253)
    at org.apache.cxf.transport.servlet.ServletController.invokeDestination(ServletController.java:234)
    at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:208)
    at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:160)
    at org.apache.cxf.transport.servlet.CXFNonSpringServlet.invoke(CXFNonSpringServlet.java:180)
    at org.apache.cxf.transport.servlet.AbstractHTTPServlet.handleRequest(AbstractHTTPServlet.java:298)
    at org.apache.cxf.transport.servlet.AbstractHTTPServlet.doPost(AbstractHTTPServlet.java:217)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
    at org.apache.cxf.transport.servlet.AbstractHTTPServlet.service(AbstractHTTPServlet.java:273)
    at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:812)
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:587)
    at org.ops4j.pax.web.service.jetty.internal.HttpServiceServletHandler.doHandle(HttpServiceServletHandler.java:70)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
    at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:577)
    at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:223)
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1127)
    at org.ops4j.pax.web.service.jetty.internal.HttpServiceContext.doHandle(HttpServiceContext.java:271)
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:515)
    at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1061)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
    at org.ops4j.pax.web.service.jetty.internal.JettyServerHandlerCollection.handle(JettyServerHandlerCollection.java:80)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
    at org.eclipse.jetty.server.Server.handle(Server.java:499)
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:311)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257)
    at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:544)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555)
    at java.lang.Thread.run(Thread.java:745)
Caused by: java.io.IOException: java.io.IOException: Exit code: 128 for command: [git, merge, --no-edit, --no-ff, pullRequest]
    at org.pentaho.build.buddy.bundles.source.github.GithubSourceRetriever.retrieveSource(GithubSourceRetriever.java:117)
    at Proxyc906d380_6153_4886_b1f7_09cfc0aa32c8.retrieveSource(Unknown Source)
    at org.pentaho.build.buddy.bundles.orchestrator.OrchestratorImpl.orchestrate(OrchestratorImpl.java:295)
    ... 41 more
Caused by: java.io.IOException: Exit code: 128 for command: [git, merge, --no-edit, --no-ff, pullRequest]
    at org.pentaho.build.buddy.util.shell.ShellUtil.executeAndCheck(ShellUtil.java:59)
    at org.pentaho.build.buddy.bundles.source.github.GithubSourceRetriever.retrieveSource(GithubSourceRetriever.java:103)
    ... 43 more

mr9d · 2016-06-29T13:51:50Z

This is done.

mr9d · 2016-06-30T12:53:24Z

@mkambol could you please merge?

mkambol reviewed Mar 3, 2016
View reviewed changes

mkambol self-assigned this Jun 23, 2016

[MONDRIAN-2451] Mondrian generates ORDER BY sql that is invalid with …

a16bbb0

…default settings of MySQL 5.7.5+ - UTs fixes for mondrian.rolap.aggregates.Use=true and mondrian.rolap.aggregates.Read=true

mkambol merged commit 162767e into pentaho:master Jun 30, 2016

mkambol mentioned this pull request Jul 1, 2016

Revert "[MONDRIAN-2451] Mondrian generates ORDER BY sql that is invalid with default settings of MySQL 5.7.5+" #727

Merged

mr9d mentioned this pull request Jul 11, 2016

[MONDRIAN-2451] Mondrian generates ORDER BY sql that is invalid with default settings of MySQL 5.7.5+ #730

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MONDRIAN-2451] Mondrian generates ORDER BY sql that is invalid with default settings of MySQL 5.7.5+ #669

[MONDRIAN-2451] Mondrian generates ORDER BY sql that is invalid with default settings of MySQL 5.7.5+ #669

mr9d commented Mar 3, 2016

mr9d commented Mar 3, 2016

mkambol Mar 3, 2016

mr9d Mar 3, 2016

mkambol commented Mar 3, 2016

mkambol commented Mar 3, 2016

lucboudreau commented Mar 3, 2016

mkambol commented Mar 3, 2016

mr9d commented Mar 4, 2016

mkambol commented Mar 4, 2016

mkambol commented Jun 21, 2016

mr9d commented Jun 23, 2016

wingman-pentaho commented Jun 28, 2016

mr9d commented Jun 28, 2016

mkambol commented Jun 28, 2016

wingman-pentaho commented Jun 29, 2016

mr9d commented Jun 29, 2016

mr9d commented Jun 30, 2016

[MONDRIAN-2451] Mondrian generates ORDER BY sql that is invalid with default settings of MySQL 5.7.5+ #669

[MONDRIAN-2451] Mondrian generates ORDER BY sql that is invalid with default settings of MySQL 5.7.5+ #669

Conversation

mr9d commented Mar 3, 2016

mr9d commented Mar 3, 2016

mkambol Mar 3, 2016

Choose a reason for hiding this comment

mr9d Mar 3, 2016

Choose a reason for hiding this comment

mkambol commented Mar 3, 2016

mkambol commented Mar 3, 2016

lucboudreau commented Mar 3, 2016

mkambol commented Mar 3, 2016

mr9d commented Mar 4, 2016

mkambol commented Mar 4, 2016

mkambol commented Jun 21, 2016

mr9d commented Jun 23, 2016

wingman-pentaho commented Jun 28, 2016

Build Failed

Stdout log

(last 100 lines)

Stderr log

(last 100 lines)

mr9d commented Jun 28, 2016

mkambol commented Jun 28, 2016

wingman-pentaho commented Jun 29, 2016

Build Failed

Stdout log

(last 100 lines)

Stderr log

(last 100 lines)

mr9d commented Jun 29, 2016

mr9d commented Jun 30, 2016