MONDRIAN: Make default expression mandatory for parameters (even if t…

…he default expression always returns the NULL value), and fix system and mondrian properties. [git-p4: depot-paths = "//open/mondrian/": change = 7502]
pentaho · Aug 29, 2006 · c137ac1 · c137ac1
1 parent 362977f
commit c137ac1
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 21 deletions.
diff --git a/src/main/mondrian/olap/MondrianProperties.java b/src/main/mondrian/olap/MondrianProperties.java
@@ -14,6 +14,7 @@
 
 import org.apache.log4j.Logger;
 import org.eigenbase.util.property.*;
+import org.eigenbase.util.property.Property;
 
 import java.io.*;
 import java.net.MalformedURLException;
@@ -266,7 +267,7 @@ private void load(final PropertySource source) {
     }
 
     /**
-     * Returns a list of all properties.
+     * Returns a list of every {@link org.eigenbase.util.property.Property}.
      *
      * <p>todo: Move to base class, {@link TriggerableProperties}, and rename
      * base method {@link TriggerableProperties#getProperties()}}.
@@ -277,7 +278,8 @@ public List getPropertyList() {
         for (int i = 0; i < fields.length; i++) {
             Field field = fields[i];
             if (!Modifier.isStatic(field.getModifiers()) &&
-                    Property.class.isAssignableFrom(field.getType())) {
+                    org.eigenbase.util.property.Property.class.isAssignableFrom(
+                        field.getType())) {
                 try {
                     list.add(field.get(this));
                 } catch (IllegalAccessException e) {
@@ -289,6 +291,27 @@ public List getPropertyList() {
         }
         return list;
     }
+
+    /**
+     * Returns the definition of a named property, or null if there is no
+     * such property.
+     *
+     * <p>todo: Move to base class, {@link TriggerableProperties}.
+     *
+     * @param path Name of the property
+     */
+    public Property getPropertyDefinition(String path) {
+        final List propertyList = getPropertyList();
+        for (int i = 0; i < propertyList.size(); i++) {
+            org.eigenbase.util.property.Property property =
+                (Property) propertyList.get(i);
+            if (property.getPath().equals(path)) {
+                return property;
+            }
+        }
+        return null;
+    }
+
     /**
      * Maximum number of simultaneous queries the system will allow.
      *

diff --git a/src/main/mondrian/olap/Parameter.java b/src/main/mondrian/olap/Parameter.java
@@ -33,7 +33,7 @@ public interface Parameter {
 
     /**
      * Returns the expression which provides the default value for this
-     * Parameter.
+     * Parameter. Never null.
      */
     Exp getDefaultExp();
 

diff --git a/src/main/mondrian/olap/ParameterImpl.java b/src/main/mondrian/olap/ParameterImpl.java
@@ -46,6 +46,7 @@ protected ParameterImpl(
         this.defaultExp = defaultExp;
         this.description = description;
         this.type = type;
+        assert defaultExp != null;
         assert type instanceof StringType ||
             type instanceof NumericType ||
             type instanceof MemberType;
@@ -122,10 +123,14 @@ public void setDescription(String description) {
     }
 
     public void setType(Type type) {
+        assert type instanceof StringType ||
+            type instanceof NumericType ||
+            type instanceof MemberType;
         this.type = type;
     }
 
     public void setDefaultExp(Exp defaultExp) {
+        assert defaultExp != null;
         this.defaultExp = defaultExp;
     }
 

diff --git a/src/main/mondrian/olap/Query.java b/src/main/mondrian/olap/Query.java
@@ -447,9 +447,9 @@ public Object visit(UnresolvedFunCall call) {
                             ParameterFunDef.getParameterType(call.getArgs());
 
                         // Create a temporary parameter. We don't know its
-                        // type yet.
+                        // type yet. The default of NULL is temporary.
                         Parameter parameter = new ParameterImpl(
-                            parameterName, null, null, type);
+                            parameterName, Literal.nullValue, null, type);
                         parameters.add(parameter);
                         parametersByName.put(parameterName, parameter);
                     }

diff --git a/src/main/mondrian/rolap/RolapSchemaReader.java b/src/main/mondrian/rolap/RolapSchemaReader.java
@@ -31,6 +31,7 @@
 import mondrian.calc.impl.GenericCalc;
 
 import org.apache.log4j.Logger;
+import org.eigenbase.util.property.Property;
 
 /**
  * A <code>RolapSchemaReader</code> allows you to read schema objects while
@@ -456,8 +457,9 @@ public Parameter getParameter(String name) {
         // Scan through mondrian and system properties.
         List propertyList = MondrianProperties.instance().getPropertyList();
         for (int i = 0; i < propertyList.size(); i++) {
-            Property property = (Property) propertyList.get(i);
-            if (property.getName().equals(name)) {
+            org.eigenbase.util.property.Property property =
+                (org.eigenbase.util.property.Property) propertyList.get(i);
+            if (property.getPath().equals(name)) {
                 return new SystemPropertyParameter(name, false);
             }
         }
@@ -493,13 +495,20 @@ private static class SystemPropertyParameter
          * false if source is a mondrian property.
          */
         private final boolean system;
+        /**
+         * Definition of mondrian property, or null if system property.
+         */
+        private final Property propertyDefinition;
 
         public SystemPropertyParameter(String name, boolean system) {
             super(name,
-                null, 
+                Literal.nullValue,
                 "System property '" + name + "'",
                 new StringType());
             this.system = system;
+            this.propertyDefinition =
+                system ? null :
+                MondrianProperties.instance().getPropertyDefinition(name);
         }
 
         public Scope getScope() {
@@ -517,11 +526,11 @@ public Calc[] getCalcs() {
                 }
 
                 public Object evaluate(Evaluator evaluator) {
-                    final String name = SystemPropertyParameter.this.getName();
                     if (system) {
+                        final String name = SystemPropertyParameter.this.getName();
                         return System.getProperty(name);
                     } else {
-                        return MondrianProperties.instance().getProperty(name);
+                        return propertyDefinition.stringValue();
                     }
                 }
             };

diff --git a/testsrc/main/mondrian/test/ParameterTest.java b/testsrc/main/mondrian/test/ParameterTest.java
@@ -34,17 +34,16 @@ public ParameterTest(String name) {
 
     // -- Helper methods ----------
 
-    private void assertSetPropertyFails(String propName) {
+    private void assertSetPropertyFails(String propName, String scope) {
         Query q = getConnection().parseQuery("select from [Sales]");
         try {
             q.setParameter(propName, "foo");
             fail("expected exception, trying to set " +
                 "non-overrideable property '" + propName + "'");
         } catch (Exception e) {
             assertTrue(e.getMessage().indexOf(
-                "Parameter '" +
-                propName +
-                "' (defined at 'Connection' scope) is not modifiable") >= 0);
+                "Parameter '" + propName + "' (defined at '" +
+                    scope + "' scope) is not modifiable") >= 0);
         }
     }
 
@@ -338,7 +337,7 @@ public void testConnectionPropsCannotBeOverridden() {
             String propName = ((EnumeratedValues.Value) propNames.get(i)).getName();
             if (!Arrays.asList(overrideableProps).contains(propName)) {
                 // try to override prop
-                assertSetPropertyFails(propName);
+                assertSetPropertyFails(propName, "Connection");
             }
         }
     }
@@ -351,11 +350,12 @@ public void testConnectionPropsCannotBeOverridden() {
     public void testSystemPropsGet() {
         List propertyList = MondrianProperties.instance().getPropertyList();
         for (int i = 0; i < propertyList.size(); i++) {
-            Property property = (Property) propertyList.get(i);
+            org.eigenbase.util.property.Property property =
+                (org.eigenbase.util.property.Property) propertyList.get(i);
             assertExprReturns(
                 "ParamRef(" +
-                    Util.singleQuoteString(property.getName()) + ")",
-                String.valueOf(MondrianProperties.instance().get(property)));
+                    Util.singleQuoteString(property.getPath()) + ")",
+                property.stringValue());
         }
     }
 
@@ -384,9 +384,10 @@ public void testMondrianPropsGetJava() {
     public void testSystemPropsSet() {
         List propertyList = MondrianProperties.instance().getPropertyList();
         for (int i = 0; i < propertyList.size(); i++) {
-            Property property = (Property) propertyList.get(i);
-            final String propName = property.getName();
-            assertSetPropertyFails(propName);
+            org.eigenbase.util.property.Property property =
+                (org.eigenbase.util.property.Property) propertyList.get(i);
+            final String propName = property.getPath();
+            assertSetPropertyFails(propName, "System");
         }
     }
 

diff --git a/testsrc/main/mondrian/test/TestContext.java b/testsrc/main/mondrian/test/TestContext.java
@@ -391,6 +391,9 @@ public Cell executeExprRaw(String expression) {
      */
     public void assertExprReturns(String expression, String expected) {
         final Cell cell = executeExprRaw(expression);
+        if (expected == null) {
+            expected = ""; // null values are formatted as empty string
+        }
         assertEqualsVerbose(expected, cell.getFormattedValue());
     }