You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lens.apache.org by pr...@apache.org on 2015/03/10 14:35:10 UTC
[3/3] incubator-lens git commit: LENS-393: Do query field validation
for chain ref columns as well (Amareshwari Sriramadasu via prongs)
LENS-393: Do query field validation for chain ref columns as well (Amareshwari Sriramadasu via prongs)
Project: http://git-wip-us.apache.org/repos/asf/incubator-lens/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-lens/commit/85b3d9e9
Tree: http://git-wip-us.apache.org/repos/asf/incubator-lens/tree/85b3d9e9
Diff: http://git-wip-us.apache.org/repos/asf/incubator-lens/diff/85b3d9e9
Branch: refs/heads/master
Commit: 85b3d9e9e89e26f560b4116ad16806eb2ba086c9
Parents: 93536b8
Author: Amareshwari Sriramadasu <am...@apache.org>
Authored: Tue Mar 10 19:04:29 2015 +0530
Committer: Rajat Khandelwal <pr...@apache.org>
Committed: Tue Mar 10 19:04:29 2015 +0530
----------------------------------------------------------------------
.../apache/lens/cube/parse/AliasReplacer.java | 38 +++++++++++++++-----
.../apache/lens/cube/parse/CubeTestSetup.java | 2 ++
.../lens/cube/parse/TestBaseCubeQueries.java | 19 ++++++++--
.../lens/cube/parse/TestQueryRewrite.java | 4 +--
4 files changed, 50 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/85b3d9e9/lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java
----------------------------------------------------------------------
diff --git a/lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java b/lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java
index 6ef72d3..d81fab1 100644
--- a/lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java
+++ b/lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java
@@ -99,7 +99,7 @@ class AliasReplacer implements ContextRewriter {
// Finds all queried dim-attributes and measures from cube
// If all fields in cube are not queryable together, does the validation
- // wrt to dervided cubes.
+ // wrt to derived cubes.
private void doFieldValidation(CubeQueryContext cubeql) throws SemanticException {
CubeInterface cube = cubeql.getCube();
if (cube != null) {
@@ -125,34 +125,41 @@ class AliasReplacer implements ContextRewriter {
} catch (HiveException e) {
throw new SemanticException(e);
}
- // remove chained ref columns from field valdation
+ // remove chained ref columns from field validation
Iterator<String> iter = queriedDimAttrs.iterator();
+ Set<String> chainedSrcColumns = new HashSet<String>();
while (iter.hasNext()) {
String attr = iter.next();
if (cube.getDimAttributeByName(attr) instanceof ReferencedDimAtrribute
&& ((ReferencedDimAtrribute) cube.getDimAttributeByName(attr)).isChainedColumn()) {
iter.remove();
+ ReferencedDimAtrribute rdim = (ReferencedDimAtrribute)cube.getDimAttributeByName(attr);
+ chainedSrcColumns.addAll(cube.getChainByName(rdim.getChainName()).getSourceColumns());
}
}
+ for (JoinChain chainQueried : cubeql.getJoinchains().values()) {
+ chainedSrcColumns.addAll(chainQueried.getSourceColumns());
+ }
// do validation
// Find atleast one derived cube which contains all the dimensions
// queried.
boolean derivedCubeFound = false;
for (DerivedCube dcube : dcubes) {
- if (dcube.getDimAttributeNames().containsAll(queriedDimAttrs)) {
+ if (dcube.getDimAttributeNames().containsAll(chainedSrcColumns)
+ && dcube.getDimAttributeNames().containsAll(queriedDimAttrs)) {
// remove all the measures that are covered
queriedMsrs.removeAll(dcube.getMeasureNames());
derivedCubeFound = true;
}
}
- if (!derivedCubeFound && !queriedDimAttrs.isEmpty()) {
- throw new SemanticException(ErrorMsg.FIELDS_NOT_QUERYABLE, queriedDimAttrs.toString());
+ Set<String> nonQueryableFields = getNonQueryableAttributes(cubeql);
+ if (!derivedCubeFound && !nonQueryableFields.isEmpty()) {
+ throw new SemanticException(ErrorMsg.FIELDS_NOT_QUERYABLE, nonQueryableFields.toString());
}
if (!queriedMsrs.isEmpty()) {
- // Add appropriate message to know which fields are not queryable
- // together
- if (!queriedDimAttrs.isEmpty()) {
- throw new SemanticException(ErrorMsg.FIELDS_NOT_QUERYABLE, queriedDimAttrs.toString() + " and "
+ // Add appropriate message to know which fields are not queryable together
+ if (!nonQueryableFields.isEmpty()) {
+ throw new SemanticException(ErrorMsg.FIELDS_NOT_QUERYABLE, nonQueryableFields.toString() + " and "
+ queriedMsrs.toString());
} else {
throw new SemanticException(ErrorMsg.FIELDS_NOT_QUERYABLE, queriedMsrs.toString());
@@ -162,6 +169,19 @@ class AliasReplacer implements ContextRewriter {
}
}
+ private Set<String> getNonQueryableAttributes(CubeQueryContext cubeql) {
+ Set<String> nonQueryableFields = new LinkedHashSet<String>();
+ nonQueryableFields.addAll(cubeql.getQueriedDimAttrs());
+ for (String joinChainAlias : cubeql.getJoinchains().keySet()) {
+ if (cubeql.getColumnsQueried(joinChainAlias) != null) {
+ for (String chaincol : cubeql.getColumnsQueried(joinChainAlias)) {
+ nonQueryableFields.add(joinChainAlias + "." + chaincol);
+ }
+ }
+ }
+ return nonQueryableFields;
+ }
+
private void extractTabAliasForCol(CubeQueryContext cubeql) throws SemanticException {
Set<String> columns = cubeql.getTblAliasToColumns().get(CubeQueryContext.DEFAULT_TABLE);
if (columns == null) {
http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/85b3d9e9/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java
----------------------------------------------------------------------
diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java
index 2350097..3fbdee9 100644
--- a/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java
+++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java
@@ -722,6 +722,8 @@ public class CubeTestSetup {
measures.add("msr13");
measures.add("msr14");
dimensions = new HashSet<String>();
+ dimensions.add("cityid");
+ dimensions.add("stateid");
dimensions.add("dim1");
dimensions.add("dim2");
dimensions.add("dim11");
http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/85b3d9e9/lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java
----------------------------------------------------------------------
diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java
index 78e0b80..1f03db6 100644
--- a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java
+++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java
@@ -58,11 +58,26 @@ public class TestBaseCubeQueries extends TestQueryRewrite {
Assert.assertEquals(e.getCanonicalErrorMsg().getErrorCode(), ErrorMsg.FIELDS_NOT_QUERYABLE.getErrorCode());
Assert.assertTrue(e.getMessage().contains("dim2") && e.getMessage().contains("msr1"));
- e = getSemanticExceptionInRewrite("select dim2, cityid, SUM(msr2) from basecube" + " where " + TWO_DAYS_RANGE,
+ e = getSemanticExceptionInRewrite("select cityStateCapital, SUM(msr1) from basecube" + " where " + TWO_DAYS_RANGE,
+ conf);
+ Assert.assertEquals(e.getCanonicalErrorMsg().getErrorCode(), ErrorMsg.FIELDS_NOT_QUERYABLE.getErrorCode());
+ Assert.assertTrue(e.getMessage().contains("citystatecapital") && e.getMessage().contains("msr1"));
+
+ e = getSemanticExceptionInRewrite("select cityState.name, SUM(msr1) from basecube" + " where " + TWO_DAYS_RANGE,
+ conf);
+ Assert.assertEquals(e.getCanonicalErrorMsg().getErrorCode(), ErrorMsg.FIELDS_NOT_QUERYABLE.getErrorCode());
+ Assert.assertTrue(e.getMessage().contains("citystate.name") && e.getMessage().contains("msr1"));
+
+ e = getSemanticExceptionInRewrite("select cubeState.name, SUM(msr1) from basecube" + " where " + TWO_DAYS_RANGE,
+ conf);
+ Assert.assertEquals(e.getCanonicalErrorMsg().getErrorCode(), ErrorMsg.FIELDS_NOT_QUERYABLE.getErrorCode());
+ Assert.assertTrue(e.getMessage().contains("cubestate.name") && e.getMessage().contains("msr1"));
+
+ e = getSemanticExceptionInRewrite("select dim2, countryid, SUM(msr2) from basecube" + " where " + TWO_DAYS_RANGE,
conf);
Assert.assertEquals(e.getCanonicalErrorMsg().getErrorCode(),
ErrorMsg.FIELDS_NOT_QUERYABLE.getErrorCode());
- Assert.assertTrue(e.getMessage().contains("dim2") && e.getMessage().contains("cityid"));
+ Assert.assertTrue(e.getMessage().contains("dim2") && e.getMessage().contains("countryid"));
e = getSemanticExceptionInRewrite("select newmeasure from basecube" + " where " + TWO_DAYS_RANGE, conf);
Assert.assertEquals(e.getCanonicalErrorMsg().getErrorCode(),
http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/85b3d9e9/lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryRewrite.java
----------------------------------------------------------------------
diff --git a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryRewrite.java b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryRewrite.java
index 66f160d..a1451b9 100644
--- a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryRewrite.java
+++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryRewrite.java
@@ -84,8 +84,8 @@ public abstract class TestQueryRewrite {
protected SemanticException getSemanticExceptionInRewrite(String query, Configuration conf) throws ParseException {
try {
- rewrite(query, conf);
- Assert.fail("Should have thrown exception");
+ String hql = rewrite(query, conf);
+ Assert.fail("Should have thrown exception. But rewrote the query : " + hql);
// unreachable
return null;
} catch (SemanticException e) {