Author: shawkins Date: 2012-10-01 10:47:01 -0400 (Mon, 01 Oct 2012) New Revision: 4493 Modified: trunk/engine/src/main/java/org/teiid/dqp/internal/datamgr/CapabilitiesConverter.java trunk/engine/src/main/java/org/teiid/dqp/internal/datamgr/LanguageBridgeFactory.java trunk/engine/src/main/java/org/teiid/dqp/internal/process/AuthorizationValidationVisitor.java trunk/engine/src/main/java/org/teiid/query/eval/Evaluator.java trunk/engine/src/main/java/org/teiid/query/function/FunctionDescriptor.java trunk/engine/src/main/java/org/teiid/query/function/metadata/FunctionMetadataValidator.java trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/CriteriaCapabilityValidatorVisitor.java trunk/engine/src/main/java/org/teiid/query/resolver/util/ResolverVisitor.java trunk/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java trunk/engine/src/test/java/org/teiid/query/function/metadata/TestFunctionMetadataValidator.java trunk/engine/src/test/java/org/teiid/query/processor/TestFunctionPushdown.java Log: TEIID-2234 fixing function issues Modified: trunk/engine/src/main/java/org/teiid/dqp/internal/datamgr/CapabilitiesConverter.java =================================================================== --- trunk/engine/src/main/java/org/teiid/dqp/internal/datamgr/CapabilitiesConverter.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/main/java/org/teiid/dqp/internal/datamgr/CapabilitiesConverter.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -120,14 +120,14 @@ Iterator<String> iter = functions.iterator(); while(iter.hasNext()) { String func = iter.next(); - tgtCaps.setFunctionSupport(func.toLowerCase(), true); + tgtCaps.setFunctionSupport(func, true); } } List<FunctionMethod> pushDowns = srcCaps.getPushDownFunctions(); if(pushDowns != null && pushDowns.size() > 0) { for(FunctionMethod func:pushDowns) { - tgtCaps.setFunctionSupport(func.getName().toLowerCase(), true); + tgtCaps.setFunctionSupport(func.getName(), true); } } Modified: trunk/engine/src/main/java/org/teiid/dqp/internal/datamgr/LanguageBridgeFactory.java =================================================================== --- trunk/engine/src/main/java/org/teiid/dqp/internal/datamgr/LanguageBridgeFactory.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/main/java/org/teiid/dqp/internal/datamgr/LanguageBridgeFactory.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -44,7 +44,6 @@ import org.teiid.language.SubqueryComparison.Quantifier; import org.teiid.metadata.Procedure; import org.teiid.metadata.ProcedureParameter; -import org.teiid.metadata.FunctionMethod.PushDown; import org.teiid.query.QueryPlugin; import org.teiid.query.function.FunctionDescriptor; import org.teiid.query.metadata.QueryMetadataInterface; @@ -673,9 +672,9 @@ return translate(caseExpr); } //check for translator pushdown functions, and use the name in source if possible - if (function.getFunctionDescriptor().getPushdown() == PushDown.MUST_PUSHDOWN - && function.getFunctionDescriptor().getSchema().equalsIgnoreCase(CoreConstants.SYSTEM_MODEL) - && function.getFunctionDescriptor().getMethod().getNameInSource() != null) { + if (function.getFunctionDescriptor().getMethod().getNameInSource() != null && + (CoreConstants.SYSTEM_MODEL.equals(function.getFunctionDescriptor().getSchema()) + || (function.getFunctionDescriptor().getMethod().getParent() != null && function.getFunctionDescriptor().getMethod().getParent().isPhysical())) ) { name = function.getFunctionDescriptor().getMethod().getNameInSource(); } } else { Modified: trunk/engine/src/main/java/org/teiid/dqp/internal/process/AuthorizationValidationVisitor.java =================================================================== --- trunk/engine/src/main/java/org/teiid/dqp/internal/process/AuthorizationValidationVisitor.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/main/java/org/teiid/dqp/internal/process/AuthorizationValidationVisitor.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -162,7 +162,7 @@ String schema = obj.getFunctionDescriptor().getSchema(); if (schema != null && !isSystemSchema(schema)) { Map<String, Function> map = new HashMap<String, Function>(); - map.put(schema + '.' + obj.getFunctionDescriptor().getName(), obj); + map.put(obj.getFunctionDescriptor().getFullName(), obj); validateEntitlements(PermissionType.EXECUTE, Context.FUNCTION, map); } } Modified: trunk/engine/src/main/java/org/teiid/query/eval/Evaluator.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/eval/Evaluator.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/main/java/org/teiid/query/eval/Evaluator.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -938,7 +938,7 @@ if (passing.getExpression() instanceof Function) { Function f = (Function)passing.getExpression(); //narrow optimization of json based documents to allow for lower overhead streaming - if (f.getFunctionDescriptor().getName().equalsIgnoreCase(SourceSystemFunctions.JSONTOXML)) { + if (f.getName().equalsIgnoreCase(SourceSystemFunctions.JSONTOXML)) { String rootName = (String)this.evaluate(f.getArg(0), tuple); Object lob = this.evaluate(f.getArg(1), tuple); if (rootName == null || lob == null) { @@ -1050,7 +1050,7 @@ fd.checkNotPushdown(); // Check for special lookup function - if(fd.getName().equalsIgnoreCase(FunctionLibrary.LOOKUP)) { + if(function.getName().equalsIgnoreCase(FunctionLibrary.LOOKUP)) { if(dataMgr == null) { throw new ComponentNotFoundException(QueryPlugin.Event.TEIID30342, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30342)); } Modified: trunk/engine/src/main/java/org/teiid/query/function/FunctionDescriptor.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/function/FunctionDescriptor.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/main/java/org/teiid/query/function/FunctionDescriptor.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -29,12 +29,14 @@ import java.util.Arrays; import org.teiid.api.exception.query.FunctionExecutionException; +import org.teiid.core.CoreConstants; import org.teiid.core.TeiidRuntimeException; import org.teiid.core.types.ArrayImpl; import org.teiid.core.types.BinaryType; import org.teiid.core.types.DataTypeManager; import org.teiid.core.types.TransformationException; import org.teiid.core.util.PropertiesUtils; +import org.teiid.metadata.AbstractMetadataRecord; import org.teiid.metadata.FunctionMethod; import org.teiid.metadata.FunctionMethod.Determinism; import org.teiid.metadata.FunctionMethod.PushDown; @@ -103,6 +105,13 @@ public String getName() { return this.method.getName(); } + + public String getFullName() { + if (CoreConstants.SYSTEM_MODEL.equals(this.schema)) { + return getName(); + } + return this.schema + AbstractMetadataRecord.NAME_DELIM_CHAR + getName(); + } public PushDown getPushdown() { return this.method.getPushdown(); @@ -183,7 +192,7 @@ public void checkNotPushdown() throws FunctionExecutionException { // Check for function we can't evaluate if(getPushdown() == PushDown.MUST_PUSHDOWN) { - throw new FunctionExecutionException(QueryPlugin.Event.TEIID30341, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30341, getName())); + throw new FunctionExecutionException(QueryPlugin.Event.TEIID30341, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30341, getFullName())); } } @@ -209,7 +218,7 @@ // If descriptor is missing invokable method, find this VM's descriptor // give name and types from fd if(invocationMethod == null) { - throw new FunctionExecutionException(QueryPlugin.Event.TEIID30382, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30382, getName())); + throw new FunctionExecutionException(QueryPlugin.Event.TEIID30382, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30382, getFullName())); } // Invoke the method and return the result @@ -270,9 +279,9 @@ } return importValue(result, getReturnType()); } catch(ArithmeticException e) { - throw new FunctionExecutionException(QueryPlugin.Event.TEIID30384, e, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30384, getName())); + throw new FunctionExecutionException(QueryPlugin.Event.TEIID30384, e, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30384, getFullName())); } catch(InvocationTargetException e) { - throw new FunctionExecutionException(QueryPlugin.Event.TEIID30384, e.getTargetException(), QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30384, getName())); + throw new FunctionExecutionException(QueryPlugin.Event.TEIID30384, e.getTargetException(), QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30384, getFullName())); } catch(IllegalAccessException e) { throw new FunctionExecutionException(QueryPlugin.Event.TEIID30385, e, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30385, method.toString())); } catch (TransformationException e) { @@ -317,4 +326,8 @@ this.calledWithVarArgArrayParam = calledWithVarArgArrayParam; } + public boolean isSystemFunction(String name) { + return this.getName().equalsIgnoreCase(name) && CoreConstants.SYSTEM_MODEL.equals(this.getSchema()); + } + } Modified: trunk/engine/src/main/java/org/teiid/query/function/metadata/FunctionMetadataValidator.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/function/metadata/FunctionMetadataValidator.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/main/java/org/teiid/query/function/metadata/FunctionMetadataValidator.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -150,7 +150,6 @@ public static final void validateName(String name) throws FunctionMetadataException { validateIsNotNull(name, "Name"); //$NON-NLS-1$ validateLength(name, MAX_LENGTH, "Name"); //$NON-NLS-1$ - validateNameCharacters(name, "Name"); //$NON-NLS-1$ } /** @@ -246,19 +245,6 @@ } /** - * Check that specified string uses valid allowed character set. If not, an exception is thrown using - * strName for the exception message. - * @param name String to check - * @param strName String to use in exception message - * @throws FunctionMetadataException Thrown when string uses characters not in allowed character sets - */ - private static final void validateNameCharacters(String name, String strName) throws FunctionMetadataException { - if (name.indexOf('.') > 0) { - throw new FunctionMetadataException(QueryPlugin.Event.TEIID30431, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30431,strName, '.')); - } - } - - /** * Check that specified string is valid Java identifier. If not, an exception is thrown using * strName for the exception message. * @param identifier String to check Modified: trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/CriteriaCapabilityValidatorVisitor.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/CriteriaCapabilityValidatorVisitor.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/main/java/org/teiid/query/optimizer/relational/rules/CriteriaCapabilityValidatorVisitor.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -279,7 +279,7 @@ markInvalid(obj, (obj.isImplicit()?"(implicit) ":"") + obj.getName() + " function not supported by source"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ return; } - String name = obj.getFunctionDescriptor().getName(); + String name = obj.getName(); if (CapabilitiesUtil.supports(Capability.ONLY_FORMAT_LITERALS, modelID, metadata, capFinder) && parseFormat.contains(name)) { if (!(obj.getArg(1) instanceof Constant)) { markInvalid(obj, obj.getName() + " non-literal parse format function not supported by source"); //$NON-NLS-1$ Modified: trunk/engine/src/main/java/org/teiid/query/resolver/util/ResolverVisitor.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/resolver/util/ResolverVisitor.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/main/java/org/teiid/query/resolver/util/ResolverVisitor.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -558,7 +558,7 @@ fd.setCalledWithVarArgArrayParam(true); } - if(fd.getName().equalsIgnoreCase(FunctionLibrary.CONVERT) || fd.getName().equalsIgnoreCase(FunctionLibrary.CAST)) { + if(fd.isSystemFunction(FunctionLibrary.CONVERT) || fd.isSystemFunction(FunctionLibrary.CAST)) { String dataType = (String) ((Constant)args[1]).getValue(); Class<?> dataTypeClass = DataTypeManager.getDataTypeClass(dataType); fd = library.findTypedConversionFunction(args[0].getType(), dataTypeClass); @@ -571,10 +571,10 @@ throw new QueryResolverException(QueryPlugin.Event.TEIID30071, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30071, new Object[] {DataTypeManager.getDataTypeName(srcTypeClass), dataType})); } - } else if(fd.getName().equalsIgnoreCase(FunctionLibrary.LOOKUP)) { + } else if(fd.isSystemFunction(FunctionLibrary.LOOKUP)) { ResolverUtil.ResolvedLookup lookup = ResolverUtil.resolveLookup(function, metadata); fd = library.copyFunctionChangeReturnType(fd, lookup.getReturnElement().getType()); - } else if (FunctionLibrary.ARRAY_GET.equalsIgnoreCase(fd.getName()) && args[0].getType().isArray()) { + } else if (fd.isSystemFunction(FunctionLibrary.ARRAY_GET) && args[0].getType().isArray()) { //hack to use typed array values fd = library.copyFunctionChangeReturnType(fd, args[0].getType().getComponentType()); } else if (Boolean.valueOf(fd.getMethod().getProperty(TEIID_PASS_THROUGH_TYPE, false))) { Modified: trunk/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -311,7 +311,7 @@ } catch (TeiidProcessingException e) { handleException(e, obj); } - } else if (obj.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.CONTEXT)) { + } else if (obj.getName().equalsIgnoreCase(FunctionLibrary.CONTEXT)) { if(!isXML) { // can't use this pseudo-function in non-XML queries handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.The_context_function_cannot_be_used_in_a_non-XML_command"), obj); //$NON-NLS-1$ @@ -323,13 +323,13 @@ for (Iterator<Function> functions = FunctionCollectorVisitor.getFunctions(obj.getArg(1), false).iterator(); functions.hasNext();) { Function function = functions.next(); - if (function.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.CONTEXT)) { + if (function.getName().equalsIgnoreCase(FunctionLibrary.CONTEXT)) { handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.Context_function_nested"), obj); //$NON-NLS-1$ } } } - } else if (obj.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMIT) || - obj.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMITEXCEPTION)) { + } else if (obj.getName().equalsIgnoreCase(FunctionLibrary.ROWLIMIT) || + obj.getName().equalsIgnoreCase(FunctionLibrary.ROWLIMITEXCEPTION)) { if(isXML) { if (!(obj.getArg(0) instanceof ElementSymbol)) { // Arg must be an element symbol @@ -339,7 +339,7 @@ // can't use this pseudo-function in non-XML queries handleValidationError(QueryPlugin.Util.getString("ValidationVisitor.The_rowlimit_function_cannot_be_used_in_a_non-XML_command"), obj); //$NON-NLS-1$ } - } else if(obj.getFunctionDescriptor().getName().equalsIgnoreCase(SourceSystemFunctions.XPATHVALUE)) { + } else if(obj.getName().equalsIgnoreCase(SourceSystemFunctions.XPATHVALUE)) { // Validate the xpath value is valid if(obj.getArgs()[1] instanceof Constant) { Constant xpathConst = (Constant) obj.getArgs()[1]; @@ -349,7 +349,7 @@ handleValidationError(QueryPlugin.Util.getString("QueryResolver.invalid_xpath", e.getMessage()), obj); //$NON-NLS-1$ } } - } else if(obj.getFunctionDescriptor().getName().equalsIgnoreCase(SourceSystemFunctions.TO_BYTES) || obj.getFunctionDescriptor().getName().equalsIgnoreCase(SourceSystemFunctions.TO_CHARS)) { + } else if(obj.getName().equalsIgnoreCase(SourceSystemFunctions.TO_BYTES) || obj.getName().equalsIgnoreCase(SourceSystemFunctions.TO_CHARS)) { try { FunctionMethods.getCharset((String)((Constant)obj.getArg(1)).getValue()); } catch (IllegalArgumentException e) { @@ -961,16 +961,16 @@ Expression expr = null; if (obj.getLeftExpression() instanceof Function) { Function leftExpr = (Function)obj.getLeftExpression(); - if (leftExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMIT) || - leftExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMITEXCEPTION)) { + if (leftExpr.getName().equalsIgnoreCase(FunctionLibrary.ROWLIMIT) || + leftExpr.getName().equalsIgnoreCase(FunctionLibrary.ROWLIMITEXCEPTION)) { function = leftExpr; expr = obj.getRightExpression(); } } if (function == null && obj.getRightExpression() instanceof Function) { Function rightExpr = (Function)obj.getRightExpression(); - if (rightExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMIT) || - rightExpr.getFunctionDescriptor().getName().equalsIgnoreCase(FunctionLibrary.ROWLIMITEXCEPTION)) { + if (rightExpr.getName().equalsIgnoreCase(FunctionLibrary.ROWLIMIT) || + rightExpr.getName().equalsIgnoreCase(FunctionLibrary.ROWLIMITEXCEPTION)) { function = rightExpr; expr = obj.getLeftExpression(); } Modified: trunk/engine/src/test/java/org/teiid/query/function/metadata/TestFunctionMetadataValidator.java =================================================================== --- trunk/engine/src/test/java/org/teiid/query/function/metadata/TestFunctionMetadataValidator.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/test/java/org/teiid/query/function/metadata/TestFunctionMetadataValidator.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -93,7 +93,7 @@ } public void testValidateNameFail3() { - helpTestValidateNameFail("a.b"); //$NON-NLS-1$ + helpTestValidateName("a.b"); //$NON-NLS-1$ } public void testValidateFunction1() { Modified: trunk/engine/src/test/java/org/teiid/query/processor/TestFunctionPushdown.java =================================================================== --- trunk/engine/src/test/java/org/teiid/query/processor/TestFunctionPushdown.java 2012-10-01 14:42:22 UTC (rev 4492) +++ trunk/engine/src/test/java/org/teiid/query/processor/TestFunctionPushdown.java 2012-10-01 14:47:01 UTC (rev 4493) @@ -143,6 +143,40 @@ new String[] {"SELECT g_0.e1 FROM pm1.g1 AS g_0"}, ComparisonMode.EXACT_COMMAND_STRING); //$NON-NLS-1$ } + @Test public void testDDLMetadata1() throws Exception { + String ddl = "CREATE foreign FUNCTION sourceFunc(msg varchar) RETURNS varchar options (nameinsource 'a.sourcefunc') " + + "CREATE foreign FUNCTION b.sourceFunc(msg varchar) RETURNS varchar " + + "CREATE foreign table X (Y varchar);"; + + QueryMetadataInterface metadata = RealMetadataFactory.fromDDL(ddl, "x", "phy"); + + FakeCapabilitiesFinder capFinder = new FakeCapabilitiesFinder(); + BasicSourceCapabilities caps = TestOptimizer.getTypicalCapabilities(); + capFinder.addCapabilities("phy", caps); //$NON-NLS-1$ + + ProcessorPlan plan = helpPlan("SELECT sourceFunc(g_0.Y), phy.b.sourceFunc(g_0.Y) FROM phy.X AS g_0", metadata, null, capFinder, + new String[] {"SELECT sourceFunc(g_0.Y), phy.b.sourceFunc(g_0.Y) FROM phy.X AS g_0"}, ComparisonMode.EXACT_COMMAND_STRING); //$NON-NLS-1$ + + //ensure that the source query contains the function schemas + HardcodedDataManager dm = new HardcodedDataManager(metadata); + dm.addData("SELECT a.sourcefunc(g_0.Y), b.sourceFunc(g_0.Y) FROM X AS g_0", new List[0]); + TestProcessor.helpProcess(plan, dm, new List[0]); + } + + @Test public void testDDLMetadataNameConflict() throws Exception { + String ddl = "CREATE foreign FUNCTION \"convert\"(msg integer, type varchar) RETURNS varchar " + + "CREATE foreign table X (Y integer);"; + + QueryMetadataInterface metadata = RealMetadataFactory.fromDDL(ddl, "x", "phy"); + + FakeCapabilitiesFinder capFinder = new FakeCapabilitiesFinder(); + BasicSourceCapabilities caps = TestOptimizer.getTypicalCapabilities(); + capFinder.addCapabilities("phy", caps); //$NON-NLS-1$ + + helpPlan("select phy.convert(y, 'z') from x", metadata, null, capFinder, + new String[] {"SELECT phy.convert(g_0.Y, 'z') FROM phy.X AS g_0"}, ComparisonMode.EXACT_COMMAND_STRING); //$NON-NLS-1$ + } + @Test public void testConcat2() throws Exception { QueryMetadataInterface metadata = RealMetadataFactory.example1Cached();