From 8d0ca7bef282358aadfd1986b5f22763435eca07 Mon Sep 17 00:00:00 2001 From: Julian Hyde Date: Fri, 6 Apr 2012 22:24:30 +0000 Subject: [PATCH] Fix bug 3515404, "Inconsistent parsing behavior('.CHILDREN' and '.Children')". git-svn-id: https://olap4j.svn.sourceforge.net/svnroot/olap4j/trunk@526 c6a108a4-781c-0410-a6c6-c2d559e19af0 --- .../mdx/parser/impl/DefaultMdxParserImpl.java | 7 +- src/org/olap4j/mdx/parser/impl/MdxParser.jj | 90 ++----------------- testsrc/org/olap4j/test/ParserTest.java | 65 ++++++++++++-- 3 files changed, 66 insertions(+), 96 deletions(-) diff --git a/src/org/olap4j/mdx/parser/impl/DefaultMdxParserImpl.java b/src/org/olap4j/mdx/parser/impl/DefaultMdxParserImpl.java index f91e95e..2df6da4 100644 --- a/src/org/olap4j/mdx/parser/impl/DefaultMdxParserImpl.java +++ b/src/org/olap4j/mdx/parser/impl/DefaultMdxParserImpl.java @@ -40,7 +40,7 @@ public class DefaultMdxParserImpl implements MdxParser { private boolean debug = false; private final FunTable funTable = new FunTable() { public boolean isProperty(String s) { - return s.equals("CHILDREN"); + return s.equalsIgnoreCase("CHILDREN"); } }; @@ -53,7 +53,8 @@ public DefaultMdxParserImpl() { public SelectNode parseSelect(String mdx) { try { - return new MdxParserImpl(mdx, false, false).selectStatement(); + return new MdxParserImpl(mdx, debug, funTable, false) + .selectStatement(); } catch (TokenMgrError e) { throw convertException(mdx, e); } catch (ParseException e) { @@ -63,7 +64,7 @@ public SelectNode parseSelect(String mdx) { public ParseTreeNode parseExpression(String mdx) { try { - return new MdxParserImpl(mdx, false, false).expression(); + return new MdxParserImpl(mdx, debug, funTable, false).expression(); } catch (TokenMgrError e) { throw convertException(mdx, e); } catch (ParseException e) { diff --git a/src/org/olap4j/mdx/parser/impl/MdxParser.jj b/src/org/olap4j/mdx/parser/impl/MdxParser.jj index 8e833f7..886f2d0 100644 --- a/src/org/olap4j/mdx/parser/impl/MdxParser.jj +++ b/src/org/olap4j/mdx/parser/impl/MdxParser.jj @@ -39,7 +39,7 @@ import java.math.BigDecimal; /** * MDX parser, generated from MdxParser.jj. * - *

The public wrapper for this parser is {@link DefaultMdxParser}. + *

The public wrapper for this parser is {@link DefaultMdxParserImpl}. * * @author jhyde * @version $Id$ @@ -52,19 +52,11 @@ import java.math.BigDecimal; }) public class MdxParserImpl { - interface FunctionTable { - boolean isProperty(String name); - } - /* private MdxParserValidator.QueryPartFactory factory; private Statement statement; */ - private final FunctionTable funTable = new FunctionTable() { - public boolean isProperty(String name) { - return name.equals("CHILDREN"); - } - }; + private DefaultMdxParserImpl.FunTable funTable; private boolean strictValidation; private static final Comparator AXIS_NODE_COMPARATOR = @@ -75,21 +67,13 @@ public class MdxParserImpl }; public MdxParserImpl( - /* - MdxParserValidator.QueryPartFactory factory, - Statement statement, - */ String queryString, boolean debug, - /* - FunTable funTable, - */ + DefaultMdxParserImpl.FunTable funTable, boolean strictValidation) { this(new StringReader(term(queryString))); -// this.factory = factory; -// this.statement = statement; -// this.funTable = funTable; + this.funTable = funTable; this.strictValidation = strictValidation; } @@ -104,11 +88,9 @@ public class MdxParserImpl ParseTreeNode recursivelyParseExp(String s) throws ParseException { MdxParserImpl parser = new MdxParserImpl( -// factory, -// statement, s, false, -// funTable, + funTable, strictValidation); return parser.expression(); } @@ -127,12 +109,10 @@ public class MdxParserImpl private List regionList(final List nodes) { return new AbstractList() { - @Override public ParseRegion get(int index) { return nodes.get(index).getRegion(); } - @Override public int size() { return nodes.size(); } @@ -142,12 +122,10 @@ public class MdxParserImpl ParseRegion region(final ParseTreeNode... nodes) { return ParseRegion.sum( new AbstractList() { - @Override public ParseRegion get(int index) { return nodes[index].getRegion(); } - @Override public int size() { return nodes.length; } @@ -155,64 +133,6 @@ public class MdxParserImpl ); } -/* - static IdentifierNode[] toIdArray(List idList) { - if (idList == null || idList.size() == 0) { - return EmptyIdArray; - } else { - return idList.toArray(new IdentifierNode[idList.size()]); - } - } - - static ParseTreeNode[] toExpArray(List expList) { - if (expList == null || expList.size() == 0) { - return EmptyExpArray; - } else { - return expList.toArray(new ParseTreeNode[expList.size()]); - } - } - - static WithMemberNode[] toFormulaArray(List formulaList) { - if (formulaList == null || formulaList.size() == 0) { - return EmptyFormulaArray; - } else { - return formulaList.toArray(new WithMemberNode[formulaList.size()]); - } - } - - static PropertyValueNode[] toMemberPropertyArray(List mpList) { - if (mpList == null || mpList.size() == 0) { - return EmptyMemberPropertyArray; - } else { - return mpList.toArray(new PropertyValueNode[mpList.size()]); - } - } - - static ParseTreeNode[] toQueryPartArray(List qpList) { - if (qpList == null || qpList.size() == 0) { - return EmptyQueryPartArray; - } else { - return qpList.toArray(new ParseTreeNode[qpList.size()]); - } - } - - static AxisNode[] toQueryAxisArray(List qpList) { - if (qpList == null || qpList.size() == 0) { - return EmptyQueryAxisArray; - } else { - return qpList.toArray(new AxisNode[qpList.size()]); - } - } - - private static final PropertyValueNode[] EmptyMemberPropertyArray = - new PropertyValueNode[0]; - private static final ParseTreeNode[] EmptyExpArray = new ParseTreeNode[0]; - private static final WithMemberNode[] EmptyFormulaArray = new WithMemberNode[0]; - private static final IdentifierNode[] EmptyIdArray = new IdentifierNode[0]; - private static final ParseTreeNode[] EmptyQueryPartArray = new ParseTreeNode[0]; - private static final AxisNode[] EmptyQueryAxisArray = new AxisNode[0]; -*/ - private static final String DQ = '"' + ""; private static final String DQDQ = DQ + DQ; diff --git a/testsrc/org/olap4j/test/ParserTest.java b/testsrc/org/olap4j/test/ParserTest.java index 292ed4d..d218103 100644 --- a/testsrc/org/olap4j/test/ParserTest.java +++ b/testsrc/org/olap4j/test/ParserTest.java @@ -288,20 +288,20 @@ private void checkUnparse(String queryString, final String expected) { } private void assertParseQueryFails(String query, String expected) { - checkFails(createParser(), query, expected); + checkFails(createParser(), query, regexpChecker(expected)); } private void assertParseExprFails(String expr, String expected) { - checkFails(createParser(), wrapExpr(expr), expected); + checkFails(createParser(), wrapExpr(expr), regexpChecker(expected)); } - private void checkFails(MdxParser p, String query, String expected) { + private void checkFails(MdxParser p, String query, Checker checker) { final ParseRegion.RegionAndSource ras = ParseRegion.findPos(query); try { SelectNode selectNode = p.parseSelect(ras.source); fail("Must return an error, got " + selectNode); } catch (Exception e) { - checkEx(e, expected, ras); + checkEx(e, checker, ras); } } @@ -315,7 +315,7 @@ private void checkFails(MdxParser p, String query, String expected) { */ public static void checkEx( Throwable ex, - String expectedMsgPattern, + Checker expectedMsgPattern, ParseRegion.RegionAndSource ras) { String NL = TestContext.NL; @@ -422,9 +422,7 @@ public static void checkEx( throw new AssertionFailedError( "todo: add carets to sql: " + sqlWithCarets); } - if ((actualMessage == null) - || !actualMessage.matches(expectedMsgPattern)) - { + if (!expectedMsgPattern.apply(actualException)) { actualException.printStackTrace(); final String actualJavaRegexp = (actualMessage == null) ? "null" @@ -1038,6 +1036,32 @@ public void testWithAdd() { assertParseQuery(queryString, TestContext.unfold(queryString)); } + /** + * Test case for bug + * 3515404, + * "Inconsistent parsing behavior('.CHILDREN' and '.Children')". + */ + public void testChildren() { + MdxParser p = createParser(); + ParseTreeNode node = p.parseExpression("[Store].[USA].CHILDREN"); + checkChildren(node, "CHILDREN"); + + ParseTreeNode node2 = p.parseExpression("[Store].[USA].Children"); + checkChildren(node2, "Children"); + + ParseTreeNode node3 = p.parseExpression("[Store].[USA].children"); + checkChildren(node3, "children"); + } + + private void checkChildren(ParseTreeNode node, String name) { + assertTrue(node instanceof CallNode); + CallNode call = (CallNode) node; + assertEquals(name, call.getOperatorName()); + assertTrue(call.getArgList().get(0) instanceof IdentifierNode); + assertEquals("[Store].[USA]", call.getArgList().get(0).toString()); + assertEquals(1, call.getArgList().size()); + } + /** * Parses an MDX query and asserts that the result is as expected when * unparsed. @@ -1076,6 +1100,31 @@ private String wrapExpr(String expr) { + expr + "\n select from [Sales]"; } + + static Checker regexpChecker(String pattern) { + return new RegexpChecker(pattern); + } + + interface Checker { + boolean apply(Throwable e); + } + + static class RegexpChecker implements Checker { + private final String pattern; + + public RegexpChecker(String pattern) { + this.pattern = pattern; + } + + public String toString() { + return "regex(" + pattern + ")"; + } + + public boolean apply(Throwable e) { + return e.getMessage() != null + && e.getMessage().matches(pattern); + } + } } // End ParserTest.java