PDA

View Full Version : [Mondrian] [Fwd: Eigenbase perforce change 9251 for review]



John V. Sichi
05-12-2007, 06:15 PM
The change below improves the new
mondrian.rolap.ignoreInvalidMembersDuringQuery property's behavior so
that the missing members don't prevent native evaluation of non-empty
crossjoin.

SqlConstraintUtils.addMemberConstraint was previously interpreting an
empty parents list as "match everything". I changed it to "match
nothing" instead for the reason given in my checkin comments. Does
anyone know of any code which was relying on the "match everything"
behavior? No tests broke.

JVS

-------- Original Message --------
Subject: Eigenbase perforce change 9251 for review
Date: Sat, 12 May 2007 14:52:01 -0700 (PDT)
From: John V. Sichi <jsichi (AT) gmail (DOT) com>
To: Andreas Voss <tonbeller (AT) a-voss (DOT) de>, Bart Pappyn
<bppn (AT) users (DOT) sourceforge.net>, Julian Hyde <jhyde (AT) users (DOT) sourceforge.net>,
John V. Sichi <jsichi (AT) gmail (DOT) com>, Matt Campbell
<Matthew.Campbell (AT) thomson (DOT) com>, Sam Birney <sbirney (AT) jaspersoft (DOT) com>,
Zelaine Fong <zfong (AT) lucidera (DOT) com>

http://p4web.eigenbase.org/@md=d&c=6PU@//9251?ac=10

Change 9251 by jvs (AT) jvs (DOT) kotick.eigenbase on 2007/05/12 14:50:10

MONDRIAN: when invalid members are tolerated in query, don't
let their presence prevent usage of native NECJ (LER-5165),
even in the case where all members of an enumeration are
invalid; in SqlConstraintUtils, interpret an empty member
set as match-nothing, not match-anything (this is
an improvement since the set is being treated as a union,
not an intersection, but I don't know if any logic was
depending on the old behavior--no tests broke)

Affected files ...

.... //open/mondrian/src/main/mondrian/rolap/RolapNativeSet.java#25 edit
.... //open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#24 edit
.... //open/mondrian/testsrc/main/mondrian/rolap/NonEmptyTest.java#51 edit
.... //open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#107 edit

Differences ...

==== //open/mondrian/src/main/mondrian/rolap/RolapNativeSet.java#25
(ktext) ====

39c39
< * @version $Id:
//open/mondrian/src/main/mondrian/rolap/RolapNativeSet.java#24 $
---
> * @version $Id: //open/mondrian/src/main/mondrian/rolap/RolapNativeSet.java#25 $
406c406,413
< * same level.
---
> * same level (after filtering out any null members). There must be at
> * least one member to begin with (may be null). If all members are
> * nulls, then the result is a valid empty predicate.
> *
> * <p>REVIEW jvs 12-May-2007: but according to the code, if
> * strict is false, then the argument is valid even if calculated
> * members are presented (and then it's flagged appropriately
> * for special handling downstream).
412a420
> RolapLevel nullLevel = null;
413a422
> int nNullMembers = 0;
418c427,438
< RolapMember m = (RolapMember) ((MemberExpr)
args[i]).getMember();
---
> RolapMember m =
> (RolapMember) ((MemberExpr) args[i]).getMember();
>
> if (m.isNull()) {
> // we're going to filter out null members anyway;
> // don't choke on the fact that their level
> // doesn't match that of others
> nullLevel = m.getLevel();
> ++nNullMembers;
> continue;
> }
>
425c445
< if (i == 0) {
---
> if (level == null) {
430a451,457
> if (level == null) {
> // all members were null; use an arbitrary one of the
> // null levels since the SQL predicate is going to always
> // fail anyway
> assert(nullLevel != null);
> level = nullLevel;
> }
434,436c461,472
< RolapMember[] members = new RolapMember[args.length];
< for (int i = 0; i < members.length; i++) {
< members[i] = (RolapMember) ((MemberExpr)
args[i]).getMember();
---
> RolapMember[] members = new RolapMember[args.length - nNullMembers];
>
> int j = 0;
> for (int i = 0; i < args.length; ++i) {
> RolapMember m =
> (RolapMember) ((MemberExpr) args[i]).getMember();
> if (m.isNull()) {
> // filter out null members
> continue;
> }
> members[j] = m;
> ++j;
437a474
>

==== //open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#24
(ktext) ====

2c2
< // $Id:
//open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#23 $
---
> // $Id: //open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#24 $
29c29
< * @version $Id:
//open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#23 $
---
> * @version $Id: //open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#24 $
261a262,266
> // Generate a predicate which is always false in order to produce
> // the empty set. It would be smarter to avoid executing SQL at
> // all in this case, but doing it this way avoid special-case
> // evaluation code.
> sqlQuery.addWhere("(1 = 0)");

==== //open/mondrian/testsrc/main/mondrian/rolap/NonEmptyTest.java#51
(ktext) ====

49c49
< * @version $Id:
//open/mondrian/testsrc/main/mondrian/rolap/NonEmptyTest.java#50 $
---
> * @version $Id: //open/mondrian/testsrc/main/mondrian/rolap/NonEmptyTest.java#51 $
440a441,457
> /** set containing only null member should not prevent usage of native */
> public void testCjNullInEnum() {
> MondrianProperties properties = MondrianProperties.instance();
> boolean savedInvalidProp =
> properties.IgnoreInvalidMembersDuringQuery.get();
> try {
> properties.IgnoreInvalidMembersDuringQuery.set(true);
> checkNative(
> 20, 0,
> "select {[Measures].[Unit Sales]} ON COLUMNS, "
> + "NON EMPTY Crossjoin({[Gender].[All Gender].[emale]}, [Customers].[All Customers].[USA].children) ON ROWS "
> + "from [Sales] ");
> } finally {
> properties.IgnoreInvalidMembersDuringQuery.set(savedInvalidProp);
> }
> }
>

==== //open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#107
(ktext) ====

2c2
< // $Id:
//open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#106 $
---
> // $Id: //open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#107 $
35c35
< * @version $Id:
//open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#106 $
---
> * @version $Id: //open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#107 $
5234a5235,5241
> String mdx2 =
> "select {[Measures].[Unit Sales]} on columns,\n" +
> "nonemptycrossjoin(\n" +
> "{[Time].[1997].[Q1], [Time].[1997].[QTOO]},\n" +
> "[Customers].[All Customers].[USA].children) on rows\n" +
> "from [Sales]";
>
5244c5251,5254
< boolean saved = properties.IgnoreInvalidMembersDuringQuery.get();
---
> boolean savedInvalidProp =
> properties.IgnoreInvalidMembersDuringQuery.get();
> String savedAlertProp =
> properties.AlertNativeEvaluationUnsupported.get();
5256a5267,5284
>
> // Verify that invalid members in query do NOT prevent
> // usage of native NECJ (LER-5165).
> properties.AlertNativeEvaluationUnsupported.set("ERROR");
> assertQueryReturns(
> mdx2,
> fold(
> "Axis #0:\n" +
> "{}\n" +
> "Axis #1:\n" +
> "{[Measures].[Unit Sales]}\n" +
> "Axis #2:\n" +
> "{[Time].[1997].[Q1], [Customers].[All Customers].[USA].[CA]}\n" +
> "{[Time].[1997].[Q1], [Customers].[All Customers].[USA].[OR]}\n" +
> "{[Time].[1997].[Q1], [Customers].[All Customers].[USA].[WA]}\n" +
> "Row #0: 16,890\n" +
> "Row #1: 19,287\n" +
> "Row #2: 30,114\n"));
5258c5286,5287
< properties.IgnoreInvalidMembersDuringQuery.set(saved);
---
> properties.IgnoreInvalidMembersDuringQuery.set(savedInvalidProp);
> properties.AlertNativeEvaluationUnsupported.set(savedAlertProp);

_______________________________________________
Mondrian mailing list
Mondrian (AT) pentaho (DOT) org
http://lists.pentaho.org/mailman/listinfo/mondrian

Julian Hyde
05-13-2007, 06:02 PM
That looks OK.

I agree that in SqlConstraintUtils, "match nothing" seems to make more
sense than "match everything".

When you were writing the code in RolapNativeSet, were you seeing null
members (that is, members for which Member.isNull() returns true)?

Many of the MDX operators implicitly drop null members from sets. For
example, in the case of the "{...}" operator, that constructs and unions
sets, {[Gender].Parent, [Gender].[M], [Gender].[F].PrevMember} returns a
set of length 1. Your code could perhaps have asserted that none of the
members was null, rather than defensively handling null members.

Julian

> -----Original Message-----
> From: mondrian-bounces (AT) pentaho (DOT) org
> [mailto:mondrian-bounces (AT) pentaho (DOT) org] On Behalf Of John V. Sichi
> Sent: Saturday, May 12, 2007 3:08 PM
> To: mondrian (AT) pentaho (DOT) org
> Subject: [Mondrian] [Fwd: Eigenbase perforce change 9251 for review]
>
> The change below improves the new
> mondrian.rolap.ignoreInvalidMembersDuringQuery property's behavior so
> that the missing members don't prevent native evaluation of non-empty
> crossjoin.
>
> SqlConstraintUtils.addMemberConstraint was previously interpreting an
> empty parents list as "match everything". I changed it to "match
> nothing" instead for the reason given in my checkin comments. Does
> anyone know of any code which was relying on the "match everything"
> behavior? No tests broke.
>
> JVS
>
> -------- Original Message --------
> Subject: Eigenbase perforce change 9251 for review
> Date: Sat, 12 May 2007 14:52:01 -0700 (PDT)
> From: John V. Sichi <jsichi (AT) gmail (DOT) com>
> To: Andreas Voss <tonbeller (AT) a-voss (DOT) de>, Bart Pappyn
> <bppn (AT) users (DOT) sourceforge.net>, Julian Hyde
> <jhyde (AT) users (DOT) sourceforge.net>,
> John V. Sichi <jsichi (AT) gmail (DOT) com>, Matt Campbell
> <Matthew.Campbell (AT) thomson (DOT) com>, Sam Birney <sbirney (AT) jaspersoft (DOT) com>,
> Zelaine Fong <zfong (AT) lucidera (DOT) com>
>
> http://p4web.eigenbase.org/@md=d&c=6PU@//9251?ac=10
>
> Change 9251 by jvs (AT) jvs (DOT) kotick.eigenbase on 2007/05/12 14:50:10
>
> MONDRIAN: when invalid members are tolerated in query, don't
> let their presence prevent usage of native NECJ (LER-5165),
> even in the case where all members of an enumeration are
> invalid; in SqlConstraintUtils, interpret an empty member
> set as match-nothing, not match-anything (this is
> an improvement since the set is being treated as a union,
> not an intersection, but I don't know if any logic was
> depending on the old behavior--no tests broke)
>
> Affected files ...
>
> ...
> //open/mondrian/src/main/mondrian/rolap/RolapNativeSet.java#25 edit
> ...
> //open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.jav
> a#24 edit
> ...
> //open/mondrian/testsrc/main/mondrian/rolap/NonEmptyTest.java#51 edit
> ...
> //open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java
> #107 edit
>
> Differences ...
>
> ==== //open/mondrian/src/main/mondrian/rolap/RolapNativeSet.java#25
> (ktext) ====
>
> 39c39
> < * @version $Id:
> //open/mondrian/src/main/mondrian/rolap/RolapNativeSet.java#24 $
> ---
> > * @version $Id:
> //open/mondrian/src/main/mondrian/rolap/RolapNativeSet.java#25 $
> 406c406,413
> < * same level.
> ---
> > * same level (after filtering out any null
> members). There must be at
> > * least one member to begin with (may be null).
> If all members are
> > * nulls, then the result is a valid empty predicate.
> > *
> > * <p>REVIEW jvs 12-May-2007: but according to the code, if
> > * strict is false, then the argument is valid even
> if calculated
> > * members are presented (and then it's flagged
> appropriately
> > * for special handling downstream).
> 412a420
> > RolapLevel nullLevel = null;
> 413a422
> > int nNullMembers = 0;
> 418c427,438
> < RolapMember m = (RolapMember) ((MemberExpr)
> args[i]).getMember();
> ---
> > RolapMember m =
> > (RolapMember) ((MemberExpr)
> args[i]).getMember();
> >
> > if (m.isNull()) {
> > // we're going to filter out null
> members anyway;
> > // don't choke on the fact that their level
> > // doesn't match that of others
> > nullLevel = m.getLevel();
> > ++nNullMembers;
> > continue;
> > }
> >
> 425c445
> < if (i == 0) {
> ---
> > if (level == null) {
> 430a451,457
> > if (level == null) {
> > // all members were null; use an arbitrary
> one of the
> > // null levels since the SQL predicate is
> going to always
> > // fail anyway
> > assert(nullLevel != null);
> > level = nullLevel;
> > }
> 434,436c461,472
> < RolapMember[] members = new RolapMember[args.length];
> < for (int i = 0; i < members.length; i++) {
> < members[i] = (RolapMember) ((MemberExpr)
> args[i]).getMember();
> ---
> > RolapMember[] members = new
> RolapMember[args.length - nNullMembers];
> >
> > int j = 0;
> > for (int i = 0; i < args.length; ++i) {
> > RolapMember m =
> > (RolapMember) ((MemberExpr)
> args[i]).getMember();
> > if (m.isNull()) {
> > // filter out null members
> > continue;
> > }
> > members[j] = m;
> > ++j;
> 437a474
> >
>
> ====
> //open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#24
> (ktext) ====
>
> 2c2
> < // $Id:
> //open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#23 $
> ---
> > // $Id:
> //open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#24 $
> 29c29
> < * @version $Id:
> //open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#23 $
> ---
> > * @version $Id:
> //open/mondrian/src/main/mondrian/rolap/SqlConstraintUtils.java#24 $
> 261a262,266
> > // Generate a predicate which is always false
> in order to produce
> > // the empty set. It would be smarter to avoid
> executing SQL at
> > // all in this case, but doing it this way
> avoid special-case
> > // evaluation code.
> > sqlQuery.addWhere("(1 = 0)");
>
> ==== //open/mondrian/testsrc/main/mondrian/rolap/NonEmptyTest.java#51
> (ktext) ====
>
> 49c49
> < * @version $Id:
> //open/mondrian/testsrc/main/mondrian/rolap/NonEmptyTest.java#50 $
> ---
> > * @version $Id:
> //open/mondrian/testsrc/main/mondrian/rolap/NonEmptyTest.java#51 $
> 440a441,457
> > /** set containing only null member should not prevent
> usage of native */
> > public void testCjNullInEnum() {
> > MondrianProperties properties =
> MondrianProperties.instance();
> > boolean savedInvalidProp =
> > properties.IgnoreInvalidMembersDuringQuery.get();
> > try {
> > properties.IgnoreInvalidMembersDuringQuery.set(true);
> > checkNative(
> > 20, 0,
> > "select {[Measures].[Unit Sales]} ON COLUMNS, "
> > + "NON EMPTY Crossjoin({[Gender].[All
> Gender].[emale]}, [Customers].[All Customers].[USA].children)
> ON ROWS "
> > + "from [Sales] ");
> > } finally {
> >
> properties.IgnoreInvalidMembersDuringQuery.set(savedInvalidProp);
> > }
> > }
> >
>
> ====
> //open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#107
> (ktext) ====
>
> 2c2
> < // $Id:
> //open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#106 $
> ---
> > // $Id:
> //open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#107 $
> 35c35
> < * @version $Id:
> //open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#106 $
> ---
> > * @version $Id:
> //open/mondrian/testsrc/main/mondrian/test/BasicQueryTest.java#107 $
> 5234a5235,5241
> > String mdx2 =
> > "select {[Measures].[Unit Sales]} on columns,\n" +
> > "nonemptycrossjoin(\n" +
> > "{[Time].[1997].[Q1], [Time].[1997].[QTOO]},\n" +
> > "[Customers].[All Customers].[USA].children) on
> rows\n" +
> > "from [Sales]";
> >
> 5244c5251,5254
> < boolean saved =
> properties.IgnoreInvalidMembersDuringQuery.get();
> ---
> > boolean savedInvalidProp =
> > properties.IgnoreInvalidMembersDuringQuery.get();
> > String savedAlertProp =
> > properties.AlertNativeEvaluationUnsupported.get();
> 5256a5267,5284
> >
> > // Verify that invalid members in query do NOT prevent
> > // usage of native NECJ (LER-5165).
> >
> properties.AlertNativeEvaluationUnsupported.set("ERROR");
> > assertQueryReturns(
> > mdx2,
> > fold(
> > "Axis #0:\n" +
> > "{}\n" +
> > "Axis #1:\n" +
> > "{[Measures].[Unit Sales]}\n" +
> > "Axis #2:\n" +
> > "{[Time].[1997].[Q1], [Customers].[All
> Customers].[USA].[CA]}\n" +
> > "{[Time].[1997].[Q1], [Customers].[All
> Customers].[USA].[OR]}\n" +
> > "{[Time].[1997].[Q1], [Customers].[All
> Customers].[USA].[WA]}\n" +
> > "Row #0: 16,890\n" +
> > "Row #1: 19,287\n" +
> > "Row #2: 30,114\n"));
> 5258c5286,5287
> < properties.IgnoreInvalidMembersDuringQuery.set(saved);
> ---
> >
> properties.IgnoreInvalidMembersDuringQuery.set(savedInvalidProp);
> >
> properties.AlertNativeEvaluationUnsupported.set(savedAlertProp);
>
> _______________________________________________
> Mondrian mailing list
> Mondrian (AT) pentaho (DOT) org
> http://lists.pentaho.org/mailman/listinfo/mondrian
>

_______________________________________________
Mondrian mailing list
Mondrian (AT) pentaho (DOT) org
http://lists.pentaho.org/mailman/listinfo/mondrian

John V. Sichi
05-13-2007, 06:19 PM
Julian Hyde wrote:
> When you were writing the code in RolapNativeSet, were you seeing null
> members (that is, members for which Member.isNull() returns true)?

Yes. I was intentionally looking for the null members since my earlier
change was putting them there in place of invalid members.

> Many of the MDX operators implicitly drop null members from sets. For
> example, in the case of the "{...}" operator, that constructs and unions
> sets, {[Gender].Parent, [Gender].[M], [Gender].[F].PrevMember} returns a
> set of length 1. Your code could perhaps have asserted that none of the
> members was null, rather than defensively handling null members.

That's what I thought when I was making the original change. But it
turns out that the {...} operator doesn't actually drop the null members
until evaluation time. The test for applicability of native NECJ
happens earlier than that, so it has to filter them out also. The
alternative would be to do some kind of rewrite earlier as a prefilter
to simplify both evaluation paths.

JVS
_______________________________________________
Mondrian mailing list
Mondrian (AT) pentaho (DOT) org
http://lists.pentaho.org/mailman/listinfo/mondrian