Repository: atlas Updated Branches: refs/heads/master 6b4c3aa7e -> 4c393d6c8
ATLAS-2356: error handling improvements in DSL Signed-off-by: Madhan Neethiraj <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/atlas/repo Commit: http://git-wip-us.apache.org/repos/asf/atlas/commit/4c393d6c Tree: http://git-wip-us.apache.org/repos/asf/atlas/tree/4c393d6c Diff: http://git-wip-us.apache.org/repos/asf/atlas/diff/4c393d6c Branch: refs/heads/master Commit: 4c393d6c85baff4687b90b67b74bae4d19f15362 Parents: 6b4c3aa Author: Ashutosh Mestry <[email protected]> Authored: Thu Jan 11 22:30:37 2018 -0800 Committer: Madhan Neethiraj <[email protected]> Committed: Sat Jan 13 12:17:49 2018 -0800 ---------------------------------------------------------------------- .../java/org/apache/atlas/AtlasErrorCode.java | 2 +- .../java/org/apache/atlas/query/AtlasDSL.java | 47 +++++-- .../atlas/query/GremlinQueryComposer.java | 135 ++++++++++++------- .../apache/atlas/query/IdentifierHelper.java | 111 +++++++++------ .../java/org/apache/atlas/query/Lookup.java | 7 +- .../apache/atlas/query/RegistryBasedLookup.java | 79 ++++++----- .../atlas/query/SelectClauseComposer.java | 20 ++- .../org/apache/atlas/query/DSLQueriesTest.java | 41 ++++-- .../atlas/query/GremlinQueryComposerTest.java | 57 +++++--- 9 files changed, 318 insertions(+), 181 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/atlas/blob/4c393d6c/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java ---------------------------------------------------------------------- diff --git a/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java index 0953725..f8829b5 100644 --- a/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java +++ b/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java @@ -103,7 +103,7 @@ public enum AtlasErrorCode { SAVED_SEARCH_CHANGE_USER(400, "ATLAS-400-00-056", "saved-search {0} can not be moved from user {1} to {2}"), INVALID_QUERY_PARAM_LENGTH(400, "ATLAS-400-00-057" , "Length of query param {0} exceeds the limit"), INVALID_QUERY_LENGTH(400, "ATLAS-400-00-058" , "Invalid query length, update {0} to change the limit" ), - INVALID_DSL_QUERY(400, "ATLAS-400-00-059" , "Invalid DSL query: {0} Reason: {1}. Please refer to Atlas DSL grammar for more information" ), + INVALID_DSL_QUERY(400, "ATLAS-400-00-059" , "Invalid DSL query: {0} | Reason: {1}. Please refer to Atlas DSL grammar for more information" ), // All Not found enums go here TYPE_NAME_NOT_FOUND(404, "ATLAS-404-00-001", "Given typename {0} was invalid"), http://git-wip-us.apache.org/repos/asf/atlas/blob/4c393d6c/repository/src/main/java/org/apache/atlas/query/AtlasDSL.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/AtlasDSL.java b/repository/src/main/java/org/apache/atlas/query/AtlasDSL.java index 7817f91..61a34b1 100644 --- a/repository/src/main/java/org/apache/atlas/query/AtlasDSL.java +++ b/repository/src/main/java/org/apache/atlas/query/AtlasDSL.java @@ -29,6 +29,8 @@ import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.query.antlr4.AtlasDSLLexer; import org.apache.atlas.query.antlr4.AtlasDSLParser; import org.apache.atlas.type.AtlasTypeRegistry; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,7 +60,6 @@ public class AtlasDSL { static AtlasDSLParser.QueryContext parse(String queryStr) throws AtlasBaseException { AtlasDSLParser.QueryContext ret; try { - InputStream stream = new ByteArrayInputStream(queryStr.getBytes()); AtlasDSLLexer lexer = new AtlasDSLLexer(CharStreams.fromStream(stream)); Validator validator = new Validator(); @@ -81,7 +82,6 @@ public class AtlasDSL { return ret; } - } static class Validator extends BaseErrorListener { @@ -108,25 +108,50 @@ public class AtlasDSL { private final AtlasTypeRegistry typeRegistry; private final int offset; private final int limit; + private final String query; public Translator(String query, AtlasTypeRegistry typeRegistry, int offset, int limit) throws AtlasBaseException { - this(Parser.parse(query), typeRegistry, offset, limit); - } - - private Translator(final AtlasDSLParser.QueryContext queryContext, AtlasTypeRegistry typeRegistry, int offset, int limit) { - this.queryContext = queryContext; + this.query = query; + this.queryContext = Parser.parse(query); this.typeRegistry = typeRegistry; this.offset = offset; this.limit = limit; } - public GremlinQuery translate() { - QueryMetadata queryMetadata = new QueryMetadata(queryContext); + public GremlinQuery translate() throws AtlasBaseException { + QueryMetadata queryMetadata = new QueryMetadata(queryContext); GremlinQueryComposer gremlinQueryComposer = new GremlinQueryComposer(typeRegistry, queryMetadata, limit, offset); DSLVisitor dslVisitor = new DSLVisitor(gremlinQueryComposer); - queryContext.accept(dslVisitor); - return new GremlinQuery(gremlinQueryComposer.get(), queryMetadata.hasSelect()); + try { + queryContext.accept(dslVisitor); + + processErrorList(gremlinQueryComposer, null); + + return new GremlinQuery(gremlinQueryComposer.get(), queryMetadata.hasSelect()); + } catch (Exception e) { + processErrorList(gremlinQueryComposer, e); + } + + return null; + } + + private void processErrorList(GremlinQueryComposer gremlinQueryComposer, Exception e) throws AtlasBaseException { + final String errorMessage; + + if (CollectionUtils.isNotEmpty(gremlinQueryComposer.getErrorList())) { + errorMessage = StringUtils.join(gremlinQueryComposer.getErrorList(), ", "); + } else { + errorMessage = e != null ? (e.getMessage() != null ? e.getMessage() : e.toString()) : null; + } + + if (errorMessage != null) { + if (e != null) { + throw new AtlasBaseException(AtlasErrorCode.INVALID_DSL_QUERY, e, this.query, errorMessage); + } + + throw new AtlasBaseException(AtlasErrorCode.INVALID_DSL_QUERY, this.query, errorMessage); + } } } http://git-wip-us.apache.org/repos/asf/atlas/blob/4c393d6c/repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java b/repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java index fe2d6d7..95ba461 100644 --- a/repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java +++ b/repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java @@ -18,8 +18,11 @@ package org.apache.atlas.query; import com.google.common.annotations.VisibleForTesting; +import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.model.discovery.SearchParameters; +import org.apache.atlas.model.typedef.AtlasStructDef; import org.apache.atlas.type.AtlasEntityType; +import org.apache.atlas.type.AtlasStructType; import org.apache.atlas.type.AtlasType; import org.apache.atlas.type.AtlasTypeRegistry; import org.apache.commons.lang.StringUtils; @@ -29,13 +32,7 @@ import org.slf4j.LoggerFactory; import java.text.DateFormat; import java.text.ParseException; import java.text.SimpleDateFormat; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.TimeZone; +import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -47,7 +44,6 @@ public class GremlinQueryComposer { private final int DEFAULT_QUERY_RESULT_LIMIT = 25; private final int DEFAULT_QUERY_RESULT_OFFSET = 0; - private final List<String> errorList = new ArrayList<>(); private final GremlinClauseList queryClauses = new GremlinClauseList(); private final Lookup lookup; private final boolean isNestedQuery; @@ -58,9 +54,7 @@ public class GremlinQueryComposer { private static final ThreadLocal<DateFormat> DSL_DATE_FORMAT = ThreadLocal.withInitial(() -> { DateFormat ret = new SimpleDateFormat(ISO8601_FORMAT); - ret.setTimeZone(TimeZone.getTimeZone("UTC")); - return ret; }); @@ -73,7 +67,7 @@ public class GremlinQueryComposer { } public GremlinQueryComposer(AtlasTypeRegistry typeRegistry, final AtlasDSL.QueryMetadata qmd, int limit, int offset) { this(new RegistryBasedLookup(typeRegistry), qmd, false); - this.context = new Context(errorList, lookup); + this.context = new Context(lookup); providedLimit = limit; providedOffset = offset < 0 ? DEFAULT_QUERY_RESULT_OFFSET : offset; @@ -152,24 +146,42 @@ public class GremlinQueryComposer { lhsI = getAdvice(lhs); } - if (lhsI.isDate()) { - rhs = parseDate(rhs); - } + if (StringUtils.isEmpty(lhsI.getQualifiedName())) { + if (LOG.isDebugEnabled()) { + LOG.debug("unknown identifier '" + lhsI.getRaw() + "'"); + } - rhs = addQuotesIfNecessary(rhs); - if (op == SearchParameters.Operator.LIKE) { - add(GremlinClause.TEXT_CONTAINS, lhsI.getQualifiedName(), getFixedRegEx(rhs)); - } else if (op == SearchParameters.Operator.IN) { - add(GremlinClause.HAS_OPERATOR, lhsI.getQualifiedName(), "within", rhs); + context.getErrorList().add("unknown identifier '" + lhsI.getRaw() + "'"); } else { - add(GremlinClause.HAS_OPERATOR, lhsI.getQualifiedName(), op.getSymbols()[1], rhs); + if (lhsI.isDate()) { + rhs = parseDate(rhs); + } + + rhs = addQuotesIfNecessary(rhs); + if (op == SearchParameters.Operator.LIKE) { + add(GremlinClause.TEXT_CONTAINS, lhsI.getQualifiedName(), getFixedRegEx(rhs)); + } else if (op == SearchParameters.Operator.IN) { + add(GremlinClause.HAS_OPERATOR, lhsI.getQualifiedName(), "within", rhs); + } else { + add(GremlinClause.HAS_OPERATOR, lhsI.getQualifiedName(), op.getSymbols()[1], rhs); + } + + if (org != null && org.getIntroduceType()) { + add(GremlinClause.DEDUP); + add(GremlinClause.IN, org.getEdgeLabel()); + context.registerActive(currentType); + } } + } - if (org != null && org.getIntroduceType()) { - add(GremlinClause.DEDUP); - add(GremlinClause.IN, org.getEdgeLabel()); - context.registerActive(currentType); + private String getQualifiedName(IdentifierHelper.Advice ia) { + String s = ia.getQualifiedName(); + if(StringUtils.isEmpty(s)) { + s = ia.getRaw(); + getErrorList().add(String.format("Error: %s is invalid", ia.getRaw())); } + + return s; } private String getFixedRegEx(String rhs) { @@ -205,25 +217,35 @@ public class GremlinQueryComposer { IdentifierHelper.Advice ia = getAdvice(scc.getItem(i)); if (!scc.getItem(i).equals(scc.getLabel(i))) { - context.addAlias(scc.getLabel(i), ia.getQualifiedName()); + context.addAlias(scc.getLabel(i), getQualifiedName(ia)); } - // Update the qualifiedNames and the assignment expressions - if (scc.updateAsApplicable(i, ia.getQualifiedName())) { + if (scc.updateAsApplicable(i, getQualifiedName(ia))) { continue; } + scc.isSelectNoop = hasNoopCondition(ia); + if(scc.isSelectNoop) { + return; + } + if (introduceType(ia)) { scc.isSelectNoop = !ia.hasParts(); if(ia.hasParts()) { - scc.assign(i, getAdvice(ia.get()).getQualifiedName(), GremlinClause.INLINE_GET_PROPERTY); + scc.assign(i, getQualifiedName(getAdvice(ia.get())), GremlinClause.INLINE_GET_PROPERTY); } } else { - scc.assign(i, ia.getQualifiedName(), GremlinClause.INLINE_GET_PROPERTY); + scc.assign(i, getQualifiedName(ia), GremlinClause.INLINE_GET_PROPERTY); } } } + private boolean hasNoopCondition(IdentifierHelper.Advice ia) { + return ia.isPrimitive() == false && + ia.isAttribute() == false && + context.hasAlias(ia.getRaw()); + } + public GremlinQueryComposer createNestedProcessor() { GremlinQueryComposer qp = new GremlinQueryComposer(lookup, queryMetadata, true); qp.context = this.context; @@ -288,12 +310,7 @@ public class GremlinQueryComposer { } public List<String> getErrorList() { - combineErrorLists(); - return errorList; - } - - private void combineErrorLists() { - errorList.addAll(context.getErrorList()); + return context.getErrorList(); } private String getTransformedClauses(String[] items) { @@ -327,12 +344,12 @@ public class GremlinQueryComposer { IdentifierHelper.Advice ia = getAdvice(name); if (queryMetadata.hasSelect() && queryMetadata.hasGroupBy()) { - addSelectTransformation(this.context.selectClauseComposer, ia.getQualifiedName(), isDesc); + addSelectTransformation(this.context.selectClauseComposer, getQualifiedName(ia), isDesc); } else if (queryMetadata.hasGroupBy()) { - addOrderByClause(ia.getQualifiedName(), isDesc); + addOrderByClause(getQualifiedName(ia), isDesc); moveToLast(GremlinClause.GROUP_BY); } else { - addOrderByClause(ia.getQualifiedName(), isDesc); + addOrderByClause(getQualifiedName(ia), isDesc); } } @@ -393,7 +410,7 @@ public class GremlinQueryComposer { try { return DSL_DATE_FORMAT.get().parse(s).getTime(); } catch (ParseException ex) { - errorList.add(ex.getMessage()); + context.errorList.add(ex.getMessage()); } return -1; @@ -439,7 +456,7 @@ public class GremlinQueryComposer { private boolean introduceType(IdentifierHelper.Advice ia) { if (ia.getIntroduceType()) { add(GremlinClause.OUT, ia.getEdgeLabel()); - context.registerActive(ia.getTypeName()); + context.registerActive(ia); } return ia.getIntroduceType(); @@ -475,7 +492,7 @@ public class GremlinQueryComposer { } IdentifierHelper.Advice ia = getAdvice(name); - add((!descr) ? GremlinClause.ORDER_BY : GremlinClause.ORDER_BY_DESC, ia.getQualifiedName()); + add((!descr) ? GremlinClause.ORDER_BY : GremlinClause.ORDER_BY_DESC, getQualifiedName(ia)); } private void addGroupByClause(String name) { @@ -484,7 +501,7 @@ public class GremlinQueryComposer { } IdentifierHelper.Advice ia = getAdvice(name); - add(GremlinClause.GROUP_BY, ia.getQualifiedName()); + add(GremlinClause.GROUP_BY, getQualifiedName(ia)); } public boolean hasFromClause() { @@ -590,23 +607,38 @@ public class GremlinQueryComposer { @VisibleForTesting static class Context { - private final List<String> errorList; - Lookup lookup; - Map<String, String> aliasMap = new HashMap<>(); - private AtlasType activeType; - private SelectClauseComposer selectClauseComposer; + private static final AtlasStructType UNKNOWN_TYPE = new AtlasStructType(new AtlasStructDef()); - public Context(List<String> errorList, Lookup lookup) { + private final Lookup lookup; + private final List<String> errorList = new ArrayList<>(); + private final Map<String, String> aliasMap = new HashMap<>(); + private AtlasType activeType; + private SelectClauseComposer selectClauseComposer; + + public Context(Lookup lookup) { this.lookup = lookup; - this.errorList = errorList; } public void registerActive(String typeName) { if(shouldRegister(typeName)) { - activeType = lookup.getType(typeName); + try { + activeType = lookup.getType(typeName); + + aliasMap.put(typeName, typeName); + } catch (AtlasBaseException e) { + errorList.add(e.getMessage()); + activeType = UNKNOWN_TYPE; + } } + } - aliasMap.put(typeName, typeName); + public void registerActive(IdentifierHelper.Advice advice) { + if (StringUtils.isNotEmpty(advice.getTypeName())) { + registerActive(advice.getTypeName()); + } else { + errorList.add("unknown identifier '" + advice.getRaw() + "'"); + activeType = UNKNOWN_TYPE; + } } public AtlasType getActiveType() { @@ -659,7 +691,6 @@ public class GremlinQueryComposer { } public List<String> getErrorList() { - errorList.addAll(lookup.getErrorList()); return errorList; } } http://git-wip-us.apache.org/repos/asf/atlas/blob/4c393d6c/repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java b/repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java index d8221aa..ab91800 100644 --- a/repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java +++ b/repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java @@ -18,6 +18,7 @@ package org.apache.atlas.query; +import org.apache.atlas.exception.AtlasBaseException; import org.apache.commons.lang.StringUtils; import java.util.regex.Matcher; @@ -61,7 +62,13 @@ public class IdentifierHelper { public static String getQualifiedName(org.apache.atlas.query.Lookup lookup, GremlinQueryComposer.Context context, String name) { - return lookup.getQualifiedName(context, name); + try { + return lookup.getQualifiedName(context, name); + } catch (AtlasBaseException e) { + context.getErrorList().add(String.format("Error for %s.%s: %s", context.getActiveTypeName(), name, e.getMessage())); + } + + return ""; } public static boolean isQuoted(String val) { @@ -101,7 +108,6 @@ public class IdentifierHelper { private String attributeName; private boolean isPrimitive; private String edgeLabel; - private String edgeDirection; private boolean introduceType; private boolean hasSubtypes; private String subTypes; @@ -117,18 +123,22 @@ public class IdentifierHelper { } private void update(org.apache.atlas.query.Lookup lookup, GremlinQueryComposer.Context context) { - newContext = context.isEmpty(); - if(!newContext) { - if(context.aliasMap.containsKey(this.raw)) { - raw = context.aliasMap.get(this.raw); - } + try { + newContext = context.isEmpty(); + if (!newContext) { + if (context.hasAlias(this.raw)) { + raw = context.getTypeNameFromAlias(this.raw); + } - updateParts(); - updateTypeInfo(lookup, context); - isTrait = lookup.isTraitType(context); - updateEdgeInfo(lookup, context); - introduceType = !isPrimitive() && !context.hasAlias(parts[0]); - updateSubTypes(lookup, context); + updateParts(); + updateTypeInfo(lookup, context); + isTrait = lookup.isTraitType(context); + updateEdgeInfo(lookup, context); + introduceType = !isPrimitive() && !context.hasAlias(parts[0]); + updateSubTypes(lookup, context); + } + } catch (NullPointerException ex) { + context.getErrorList().add(ex.getMessage()); } } @@ -146,50 +156,65 @@ public class IdentifierHelper { private void updateEdgeInfo(org.apache.atlas.query.Lookup lookup, GremlinQueryComposer.Context context) { if(isPrimitive == false && isTrait == false) { edgeLabel = lookup.getRelationshipEdgeLabel(context, attributeName); - edgeDirection = "OUT"; typeName = lookup.getTypeFromEdge(context, attributeName); } } private void updateTypeInfo(org.apache.atlas.query.Lookup lookup, GremlinQueryComposer.Context context) { if(parts.length == 1) { - typeName = context.getActiveTypeName(); + typeName = context.hasAlias(parts[0]) ? + context.getTypeNameFromAlias(parts[0]) : + context.getActiveTypeName(); + qualifiedName = getDefaultQualifiedNameForSinglePartName(context, parts[0]); attributeName = parts[0]; - isAttribute = lookup.hasAttribute(context, typeName); - qualifiedName = lookup.getQualifiedName(context, attributeName); - isPrimitive = lookup.isPrimitive(context, attributeName); - - setIsDate(lookup, context); } if(parts.length == 2) { - if(context.hasAlias(parts[0])) { - typeName = context.getTypeNameFromAlias(parts[0]); + boolean isAttrOfActiveType = lookup.hasAttribute(context, parts[0]); + if(isAttrOfActiveType) { + attributeName = parts[0]; + } else { + typeName = context.hasAlias(parts[0]) ? + context.getTypeNameFromAlias(parts[0]) : + parts[0]; + attributeName = parts[1]; - isPrimitive = lookup.isPrimitive(context, attributeName); - setIsDate(lookup, context); - } - else { - isAttribute = lookup.hasAttribute(context, parts[0]); - if(isAttribute) { - attributeName = parts[0]; - isPrimitive = lookup.isPrimitive(context, attributeName); - setIsDate(lookup, context); - } else { - typeName = parts[0]; - attributeName = parts[1]; - isPrimitive = lookup.isPrimitive(context, attributeName); - setIsDate(lookup, context); - } } + } + + isAttribute = lookup.hasAttribute(context, attributeName); + isPrimitive = lookup.isPrimitive(context, attributeName); + setQualifiedName(lookup, context, isAttribute, attributeName); + setIsDate(lookup, context, isPrimitive, attributeName); + } + + private String getDefaultQualifiedNameForSinglePartName(GremlinQueryComposer.Context context, String s) { + String qn = context.getTypeNameFromAlias(s); + if(StringUtils.isEmpty(qn) && SelectClauseComposer.isKeyword(s)) { + return s; + } - qualifiedName = lookup.getQualifiedName(context, attributeName); + return qn; + } + + private void setQualifiedName(Lookup lookup, GremlinQueryComposer.Context context, boolean isAttribute, String attrName) { + if(isAttribute) { + qualifiedName = getQualifiedName(lookup, context, attrName); } } - private void setIsDate(Lookup lookup, GremlinQueryComposer.Context context) { + private String getQualifiedName(Lookup lookup, GremlinQueryComposer.Context context, String name) { + try { + return lookup.getQualifiedName(context, name); + } catch (AtlasBaseException e) { + context.getErrorList().add(String.format("Error for %s.%s: %s", context.getActiveTypeName(), name, e.getMessage())); + return ""; + } + } + + private void setIsDate(Lookup lookup, GremlinQueryComposer.Context context, boolean isPrimitive, String attrName) { if(isPrimitive) { - isDate = lookup.isDate(context, attributeName); + isDate = lookup.isDate(context, attrName); } } @@ -248,5 +273,9 @@ public class IdentifierHelper { public boolean hasParts() { return parts.length > 1; } - } + + public String getRaw() { + return raw; + } + } } http://git-wip-us.apache.org/repos/asf/atlas/blob/4c393d6c/repository/src/main/java/org/apache/atlas/query/Lookup.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/Lookup.java b/repository/src/main/java/org/apache/atlas/query/Lookup.java index fa75cfb..9a7d474 100644 --- a/repository/src/main/java/org/apache/atlas/query/Lookup.java +++ b/repository/src/main/java/org/apache/atlas/query/Lookup.java @@ -18,15 +18,16 @@ package org.apache.atlas.query; +import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.type.AtlasType; import java.util.Collection; import java.util.List; public interface Lookup { - AtlasType getType(String typeName); + AtlasType getType(String typeName) throws AtlasBaseException; - String getQualifiedName(GremlinQueryComposer.Context context, String name); + String getQualifiedName(GremlinQueryComposer.Context context, String name) throws AtlasBaseException; boolean isPrimitive(GremlinQueryComposer.Context context, String attributeName); @@ -43,6 +44,4 @@ public interface Lookup { String getTypeFromEdge(GremlinQueryComposer.Context context, String item); boolean isDate(GremlinQueryComposer.Context context, String attributeName); - - List<String> getErrorList(); } http://git-wip-us.apache.org/repos/asf/atlas/blob/4c393d6c/repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java b/repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java index b78bd2e..eef3463 100644 --- a/repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java +++ b/repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java @@ -20,6 +20,7 @@ package org.apache.atlas.query; import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.model.TypeCategory; +import org.apache.atlas.model.instance.AtlasObjectId; import org.apache.atlas.model.typedef.AtlasBaseTypeDef; import org.apache.atlas.type.*; import org.apache.commons.lang.StringUtils; @@ -37,30 +38,18 @@ class RegistryBasedLookup implements Lookup { } @Override - public AtlasType getType(String typeName) { - try { - return typeRegistry.getType(typeName); - } catch (AtlasBaseException e) { - addError(e.getMessage()); - } - - return null; + public AtlasType getType(String typeName) throws AtlasBaseException { + return typeRegistry.getType(typeName); } @Override - public String getQualifiedName(GremlinQueryComposer.Context context, String name) { - try { - AtlasEntityType et = context.getActiveEntityType(); - if(et == null) { - return ""; - } - - return et.getQualifiedAttributeName(name); - } catch (AtlasBaseException e) { - addError(e.getMessage()); + public String getQualifiedName(GremlinQueryComposer.Context context, String name) throws AtlasBaseException { + AtlasEntityType et = context.getActiveEntityType(); + if (et == null) { + return ""; } - return ""; + return et.getQualifiedAttributeName(name); } @Override @@ -70,13 +59,29 @@ class RegistryBasedLookup implements Lookup { return false; } - AtlasType attr = et.getAttributeType(attributeName); - if(attr == null) { + AtlasType at = et.getAttributeType(attributeName); + if(at == null) { return false; } - TypeCategory attrTypeCategory = attr.getTypeCategory(); - return (attrTypeCategory != null) && (attrTypeCategory == TypeCategory.PRIMITIVE || attrTypeCategory == TypeCategory.ENUM); + TypeCategory tc = at.getTypeCategory(); + if (isPrimitiveUsingTypeCategory(tc)) return true; + + if ((tc != null) && (tc == TypeCategory.ARRAY)) { + AtlasArrayType ct = ((AtlasArrayType)at); + return isPrimitiveUsingTypeCategory(ct.getElementType().getTypeCategory()); + } + + if ((tc != null) && (tc == TypeCategory.MAP)) { + AtlasMapType ct = ((AtlasMapType)at); + return isPrimitiveUsingTypeCategory(ct.getValueType().getTypeCategory()); + } + + return false; + } + + private boolean isPrimitiveUsingTypeCategory(TypeCategory tc) { + return ((tc != null) && (tc == TypeCategory.PRIMITIVE || tc == TypeCategory.ENUM)); } @Override @@ -136,14 +141,27 @@ class RegistryBasedLookup implements Lookup { } AtlasType at = attr.getAttributeType(); - if(at.getTypeCategory() == TypeCategory.ARRAY) { - AtlasArrayType arrType = ((AtlasArrayType)at); - return ((AtlasBuiltInTypes.AtlasObjectIdType) arrType.getElementType()).getObjectType(); + switch (at.getTypeCategory()) { + case ARRAY: + AtlasArrayType arrType = ((AtlasArrayType)at); + return getCollectionElementType(arrType.getElementType()); + + case MAP: + AtlasMapType mapType = ((AtlasMapType)at); + return getCollectionElementType(mapType.getValueType()); } return context.getActiveEntityType().getAttribute(item).getTypeName(); } + private String getCollectionElementType(AtlasType elemType) { + if(elemType.getTypeCategory() == TypeCategory.OBJECT_ID_TYPE) { + return ((AtlasBuiltInTypes.AtlasObjectIdType)elemType).getObjectType(); + } else { + return elemType.getTypeName(); + } + } + @Override public boolean isDate(GremlinQueryComposer.Context context, String attributeName) { AtlasEntityType et = context.getActiveEntityType(); @@ -155,13 +173,4 @@ class RegistryBasedLookup implements Lookup { return attr != null && attr.getTypeName().equals(AtlasBaseTypeDef.ATLAS_TYPE_DATE); } - - protected void addError(String s) { - errorList.add(s); - } - - @Override - public List<String> getErrorList() { - return errorList; - } } http://git-wip-us.apache.org/repos/asf/atlas/blob/4c393d6c/repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java b/repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java index b93e223..b7f6d3a 100644 --- a/repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java +++ b/repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java @@ -24,6 +24,11 @@ import java.util.Map; import java.util.StringJoiner; class SelectClauseComposer { + private static final String COUNT_STR = "count"; + private static final String MIN_STR = "min"; + private static final String MAX_STR = "max"; + private static final String SUM_STR = "sum"; + public boolean isSelectNoop; private String[] labels; @@ -50,16 +55,16 @@ class SelectClauseComposer { public boolean updateAsApplicable(int currentIndex, String qualifiedName) { boolean ret = false; if (currentIndex == getCountIdx()) { - ret = assign(currentIndex, "count", + ret = assign(currentIndex, COUNT_STR, GremlinClause.INLINE_COUNT.get(), GremlinClause.INLINE_ASSIGNMENT); } else if (currentIndex == getMinIdx()) { - ret = assign(currentIndex, "min", qualifiedName, + ret = assign(currentIndex, MIN_STR, qualifiedName, GremlinClause.INLINE_ASSIGNMENT, GremlinClause.INLINE_MIN); } else if (currentIndex == getMaxIdx()) { - ret = assign(currentIndex, "max", qualifiedName, + ret = assign(currentIndex, MAX_STR, qualifiedName, GremlinClause.INLINE_ASSIGNMENT, GremlinClause.INLINE_MAX); } else if (currentIndex == getSumIdx()) { - ret = assign(currentIndex, "sum", qualifiedName, + ret = assign(currentIndex, SUM_STR, qualifiedName, GremlinClause.INLINE_ASSIGNMENT, GremlinClause.INLINE_SUM); } else { attributes[currentIndex] = qualifiedName; @@ -68,6 +73,13 @@ class SelectClauseComposer { return ret; } + public static boolean isKeyword(String s) { + return COUNT_STR.equals(s) || + MIN_STR.equals(s) || + MAX_STR.equals(s) || + SUM_STR.equals(s); + } + public String[] getAttributes() { return attributes; } http://git-wip-us.apache.org/repos/asf/atlas/blob/4c393d6c/repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java ---------------------------------------------------------------------- diff --git a/repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java b/repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java index 5db55dc..c44eea3 100644 --- a/repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java +++ b/repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java @@ -116,6 +116,7 @@ public class DSLQueriesTest extends BasicTestSetup { return new Object[][]{ {"from hive_db", 3}, {"hive_db", 3}, + {"hive_db as d select d", 3}, {"hive_db where hive_db.name=\"Reporting\"", 1}, {"hive_db where hive_db.name=\"Reporting\" select name, owner", 1}, {"hive_db has name", 3}, @@ -133,7 +134,7 @@ public class DSLQueriesTest extends BasicTestSetup { {"hive_db where hive_db is JdbcAccess", 0}, {"hive_db where hive_db has name", 3}, {"hive_db as db1 hive_table where (db1.name = \"Reporting\")", 0}, - {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01\" ) select name as _col_0, createTime as _col_1", 1}, + {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01T0:0:0.0Z\" ) select name as _col_0, createTime as _col_1", 1}, {"hive_table where (name = \"sales_fact\" and createTime >= \"2014-12-11T02:35:58.440Z\" ) select name as _col_0, createTime as _col_1", 1}, {"Dimension", 5}, {"JdbcAccess", 2}, @@ -141,8 +142,6 @@ public class DSLQueriesTest extends BasicTestSetup { {"Metric", 5}, {"PII", 4}, {"`Log Data`", 3}, - {"`isa`", 0}, - {"hive_table as t, sd, hive_column as c where t.name=\"sales_fact\" select c.name as colName, c.dataType as colType", 0}, {"DataSet where name='sales_fact'", 1}, {"Asset where name='sales_fact'", 1} }; @@ -151,7 +150,7 @@ public class DSLQueriesTest extends BasicTestSetup { @Test(dataProvider = "basicProvider") public void basic(String query, int expected) throws AtlasBaseException { queryAssert(query, expected); - //queryAssert(query.replace("where", " "), expected); + queryAssert(query.replace("where", " "), expected); } private void queryAssert(String query, int expected) throws AtlasBaseException { @@ -249,10 +248,10 @@ public class DSLQueriesTest extends BasicTestSetup { {"hive_db as db1 hive_table where (db1.name = \"Reporting\")", 0}, - {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01\" ) select name as _col_0, createTime as _col_1", 1}, - {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01\" ) select name as _col_0, createTime as _col_1 limit 10", 1}, - {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01\" ) select name as _col_0, createTime as _col_1 limit 10 offset 0", 1}, - {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01\" ) select name as _col_0, createTime as _col_1 limit 10 offset 5", 0}, + {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01T0:0:0.0Z\" ) select name as _col_0, createTime as _col_1", 1}, + {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01T0:0:0.0Z\" ) select name as _col_0, createTime as _col_1 limit 10", 1}, + {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01T0:0:0.0Z\" ) select name as _col_0, createTime as _col_1 limit 10 offset 0", 1}, + {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01T0:0:0.0Z\" ) select name as _col_0, createTime as _col_1 limit 10 offset 5", 0}, {"hive_table where (name = \"sales_fact\" and createTime >= \"2014-12-11T02:35:58.440Z\" ) select name as _col_0, createTime as _col_1", 1}, {"hive_table where (name = \"sales_fact\" and createTime >= \"2014-12-11T02:35:58.440Z\" ) select name as _col_0, createTime as _col_1 limit 10 offset 0", 1}, @@ -332,10 +331,10 @@ public class DSLQueriesTest extends BasicTestSetup { {"hive_db where hive_db has name orderby hive_db.owner limit 2 offset 0", 2, "owner", true}, {"hive_db where hive_db has name orderby hive_db.owner limit 2 offset 1", 2, "owner", true}, - {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01\" ) select name as _col_0, createTime as _col_1 orderby createTime ", 1, "_col_1", true}, - {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01\" ) select name as _col_0, createTime as _col_1 orderby createTime limit 10 ", 1, "_col_1", true}, - {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01\" ) select name as _col_0, createTime as _col_1 orderby createTime limit 10 offset 0", 1, "_col_1", true}, - {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01\" ) select name as _col_0, createTime as _col_1 orderby createTime limit 10 offset 5", 0, "_col_1", true}, + {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01T0:0:0.0Z\" ) select name as _col_0, createTime as _col_1 orderby createTime ", 1, "_col_1", true}, + {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01T0:0:0.0Z\" ) select name as _col_0, createTime as _col_1 orderby createTime limit 10 ", 1, "_col_1", true}, + {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01T0:0:0.0Z\" ) select name as _col_0, createTime as _col_1 orderby createTime limit 10 offset 0", 1, "_col_1", true}, + {"hive_table where (name = \"sales_fact\" and createTime > \"2014-01-01T0:0:0.0Z\" ) select name as _col_0, createTime as _col_1 orderby createTime limit 10 offset 5", 0, "_col_1", true}, {"hive_table where (name = \"sales_fact\" and createTime >= \"2014-12-11T02:35:58.440Z\" ) select name as _col_0, createTime as _col_1 orderby name ", 1, "_col_0", true}, {"hive_table where (name = \"sales_fact\" and createTime >= \"2014-12-11T02:35:58.440Z\" ) select name as _col_0, createTime as _col_1 orderby name limit 10 offset 0", 1, "_col_0", true}, @@ -469,8 +468,6 @@ public class DSLQueriesTest extends BasicTestSetup { new FieldValueValidator() .withFieldNames("'count'", "'sum'") .withExpectedValues(4, 86) }, - // tests to ensure that group by works with order by and limit - // FIXME: // { "from hive_db groupby (owner) select min(name) orderby name limit 2 ", // new FieldValueValidator() // .withFieldNames("min(name)") @@ -490,6 +487,22 @@ public class DSLQueriesTest extends BasicTestSetup { queryAssert(query.replace("where", " "), fv); } + @DataProvider(name = "errorQueriesProvider") + private Object[][] errorQueries() { + return new Object[][]{ + {"`isa`"}, + {"PIII"}, + {"DBBB as d select d"}, + {"hive_table where (name = \"sales_fact\" and createTime >= \"2014-12-11\" ) select name as _col_0, createTime as _col_1 orderby name limit 0 offset 1"}, + {"hive_table as t, sd, hive_column as c where t.name=\"sales_fact\" select c.name as colName, c.dataType as colType"} + }; + } + + @Test(dataProvider = "errorQueriesProvider", expectedExceptions = { AtlasBaseException.class }) + public void errorQueries(String query) throws AtlasBaseException { + discoveryService.searchUsingDslQuery(query, 25, 0); + } + private void queryAssert(String query, FieldValueValidator fv) throws AtlasBaseException { AtlasSearchResult searchResult = discoveryService.searchUsingDslQuery(query, 25, 0); assertSearchResult(searchResult, fv); http://git-wip-us.apache.org/repos/asf/atlas/blob/4c393d6c/repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java ---------------------------------------------------------------------- diff --git a/repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java b/repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java index 3fcd325..36f0bf7 100644 --- a/repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java +++ b/repository/src/test/java/org/apache/atlas/query/GremlinQueryComposerTest.java @@ -76,6 +76,7 @@ public class GremlinQueryComposerTest { "f(g.V().has('__typeName', 'DB').as('d')"; verify("DB as d select d.name, d.owner", expected + ".limit(25).toList())"); verify("DB as d select d.name, d.owner limit 10", expected + ".limit(10).toList())"); + verify("DB as d select d","def f(r){ r }; f(g.V().has('__typeName', 'DB').as('d').limit(25).toList())"); } @Test @@ -178,8 +179,8 @@ public class GremlinQueryComposerTest { @Test public void whereClauseWithDateCompare() { String exSel = "def f(r){ t=[['t.name','t.owner']]; r.each({t.add([it.value('Table.name'),it.value('Table.owner')])}); t.unique(); }"; - String exMain = "g.V().has('__typeName', 'Table').as('t').has('Table.createdTime', eq('1513046158440')).limit(25).toList()"; - verify("Table as t where t.createdTime = \"2017-12-12T02:35:58.440Z\" select t.name, t.owner)", getExpected(exSel, exMain)); + String exMain = "g.V().has('__typeName', 'Table').as('t').has('Table.createTime', eq('1513046158440')).limit(25).toList()"; + verify("Table as t where t.createTime = \"2017-12-12T02:35:58.440Z\" select t.name, t.owner)", getExpected(exSel, exMain)); } @Test @@ -277,8 +278,8 @@ public class GremlinQueryComposerTest { "__.has('Table.owner', eq(\"Joe BI\"))" + "))" + ".limit(25).toList()"}, - {"Table where owner=\"hdfs\" or ((name=\"testtable_1\" or name=\"testtable_2\") and createdTime < \"2017-12-12T02:35:58.440Z\")", - "g.V().has('__typeName', 'Table').or(__.has('Table.owner', eq(\"hdfs\")),__.and(__.or(__.has('Table.name', eq(\"testtable_1\")),__.has('Table.name', eq(\"testtable_2\"))),__.has('Table.createdTime', lt('1513046158440')))).limit(25).toList()"}, + {"Table where owner=\"hdfs\" or ((name=\"testtable_1\" or name=\"testtable_2\") and createTime < \"2017-12-12T02:35:58.440Z\")", + "g.V().has('__typeName', 'Table').or(__.has('Table.owner', eq(\"hdfs\")),__.and(__.or(__.has('Table.name', eq(\"testtable_1\")),__.has('Table.name', eq(\"testtable_2\"))),__.has('Table.createTime', lt('1513046158440')))).limit(25).toList()"}, {"hive_db where hive_db.name='Reporting' and hive_db.createTime < '2017-12-12T02:35:58.440Z'", "g.V().has('__typeName', 'hive_db').and(__.has('hive_db.name', eq('Reporting')),__.has('hive_db.createTime', lt('1513046158440'))).limit(25).toList()"}, {"Table where db.name='Sales' and db.clusterName='cl1'", @@ -293,9 +294,10 @@ public class GremlinQueryComposerTest { } @Test - public void hasInWhereClause() { + public void keywordsInWhereClause() { verify("Table as t where t has name and t isa Dimension", "g.V().has('__typeName', 'Table').as('t').and(__.has('Table.name'),__.has('__traitNames', within('Dimension'))).limit(25).toList()"); + verify("Table as t where t has name and t.name = 'sales_fact'", "g.V().has('__typeName', 'Table').as('t').and(__.has('Table.name'),__.has('Table.name', eq('sales_fact'))).limit(25).toList()"); verify("Table as t where t is Dimension and t.name = 'sales_fact'", @@ -306,6 +308,7 @@ public class GremlinQueryComposerTest { @Test public void invalidQueries() { verify("hdfs_path like h1", ""); +// verify("hdfs_path select xxx", ""); } private void verify(String dsl, String expectedGremlin) { @@ -332,7 +335,7 @@ public class GremlinQueryComposerTest { private String getGremlinQuery(AtlasDSLParser.QueryContext queryContext) { AtlasTypeRegistry registry = mock(AtlasTypeRegistry.class); org.apache.atlas.query.Lookup lookup = new TestLookup(errorList, registry); - GremlinQueryComposer.Context context = new GremlinQueryComposer.Context(errorList, lookup); + GremlinQueryComposer.Context context = new GremlinQueryComposer.Context(lookup); AtlasDSL.QueryMetadata queryMetadata = new AtlasDSL.QueryMetadata(queryContext); GremlinQueryComposer gremlinQueryComposer = new GremlinQueryComposer(lookup, context, queryMetadata); @@ -340,6 +343,7 @@ public class GremlinQueryComposerTest { qv.visit(queryContext); String s = gremlinQueryComposer.get(); + assertEquals(gremlinQueryComposer.getErrorList().size(), 0); return s; } @@ -367,18 +371,24 @@ public class GremlinQueryComposerTest { } @Override - public String getQualifiedName(GremlinQueryComposer.Context context, String name) { + public String getQualifiedName(GremlinQueryComposer.Context context, String name) throws AtlasBaseException { + if(!hasAttribute(context, name)) { + throw new AtlasBaseException("Invalid attribute"); + } + if(name.contains(".")) return name; - return String.format("%s.%s", context.getActiveTypeName(), name); + if(!context.getActiveTypeName().equals(name)) + return String.format("%s.%s", context.getActiveTypeName(), name); + else + return name; } @Override public boolean isPrimitive(GremlinQueryComposer.Context context, String attributeName) { return attributeName.equals("name") || attributeName.equals("owner") || - attributeName.equals("createdTime") || attributeName.equals("createTime") || attributeName.equals("clusterName"); } @@ -394,9 +404,22 @@ public class GremlinQueryComposerTest { } @Override - public boolean hasAttribute(GremlinQueryComposer.Context context, String typeName) { - return (context.getActiveTypeName().equals("Table") && typeName.equals("db")) || - (context.getActiveTypeName().equals("Table") && typeName.equals("columns")); + public boolean hasAttribute(GremlinQueryComposer.Context context, String attributeName) { + return (context.getActiveTypeName().equals("Table") && attributeName.equals("db")) || + (context.getActiveTypeName().equals("Table") && attributeName.equals("columns")) || + (context.getActiveTypeName().equals("Table") && attributeName.equals("createTime")) || + (context.getActiveTypeName().equals("Table") && attributeName.equals("name")) || + (context.getActiveTypeName().equals("Table") && attributeName.equals("owner")) || + (context.getActiveTypeName().equals("Table") && attributeName.equals("clusterName")) || + (context.getActiveTypeName().equals("Table") && attributeName.equals("isFile")) || + (context.getActiveTypeName().equals("hive_db") && attributeName.equals("name")) || + (context.getActiveTypeName().equals("hive_db") && attributeName.equals("owner")) || + (context.getActiveTypeName().equals("hive_db") && attributeName.equals("createTime")) || + (context.getActiveTypeName().equals("DB") && attributeName.equals("name")) || + (context.getActiveTypeName().equals("DB") && attributeName.equals("owner")) || + (context.getActiveTypeName().equals("DB") && attributeName.equals("clusterName")) || + (context.getActiveTypeName().equals("Asset") && attributeName.equals("name")) || + (context.getActiveTypeName().equals("Asset") && attributeName.equals("owner")); } @Override @@ -425,19 +448,15 @@ public class GremlinQueryComposerTest { return "DB"; } else if(context.getActiveTypeName().equals("Table") && item.equals("columns")) { return "Column"; + } else if(context.getActiveTypeName().equals(item)) { + return null; } return context.getActiveTypeName(); } @Override public boolean isDate(GremlinQueryComposer.Context context, String attributeName) { - return attributeName.equals("createdTime") || - attributeName.equals("createTime"); - } - - @Override - public List<String> getErrorList() { - return errorList; + return attributeName.equals("createTime"); } } }
