This is an automated email from the ASF dual-hosted git repository.
dlych pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git
The following commit(s) were added to refs/heads/master by this push:
new 75f1987 [NO ISSUE][COMP] Cleanup Identifier constructors
75f1987 is described below
commit 75f1987a401e43f1c76ff5362d87de79c34e3998
Author: Dmitry Lychagin <[email protected]>
AuthorDate: Thu Oct 3 17:56:35 2019 -0700
[NO ISSUE][COMP] Cleanup Identifier constructors
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- Remove default constructors from
Identifier, VarIdentifier, VariableExpr
- Make Identifier immutable
Change-Id: I3a0b4da0e2b621d309b8d9aa3c47540eb18566eb
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/3607
Contrib: Jenkins <[email protected]>
Tested-by: Jenkins <[email protected]>
Integration-Tests: Jenkins <[email protected]>
Reviewed-by: Dmitry Lychagin <[email protected]>
Reviewed-by: Ali Alsuliman <[email protected]>
---
.../src/main/javacc/AQLPlusExtension.jj | 16 ++++-------
.../asterix/app/translator/QueryTranslator.java | 29 ++++++++++++-------
.../apache/asterix/lang/aql/clause/ForClause.java | 3 --
.../lang/aql/clause/MetaVariableClause.java | 7 +++++
.../lang/aql/expression/MetaVariableExpr.java | 5 ++++
asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj | 33 ++++++++++------------
.../lang/common/expression/VariableExpr.java | 5 ----
.../asterix/lang/common/statement/DatasetDecl.java | 8 ++----
.../asterix/lang/common/struct/Identifier.java | 9 +-----
.../asterix/lang/common/struct/VarIdentifier.java | 11 ++------
.../lang/sqlpp/util/ExpressionToVariableUtil.java | 3 +-
.../asterix-lang-sqlpp/src/main/javacc/SQLPP.jj | 26 ++++++++---------
12 files changed, 69 insertions(+), 86 deletions(-)
diff --git a/asterixdb/asterix-algebra/src/main/javacc/AQLPlusExtension.jj
b/asterixdb/asterix-algebra/src/main/javacc/AQLPlusExtension.jj
index c3cfdb1..35e8ff0 100644
--- a/asterixdb/asterix-algebra/src/main/javacc/AQLPlusExtension.jj
+++ b/asterixdb/asterix-algebra/src/main/javacc/AQLPlusExtension.jj
@@ -106,30 +106,24 @@ Expression FLWOGR() throws ParseException:
@new
Clause MetaVariableClause() throws ParseException :
{
- MetaVariableClause mc = new MetaVariableClause();
- VarIdentifier var = new VarIdentifier();
}
{
<METAVARIABLECLAUSE>
{
- mc.setVar(var);
- var.setValue(token.image);
- return mc;
+ VarIdentifier var = new VarIdentifier(token.image);
+ return new MetaVariableClause(var);
}
}
@new
MetaVariableExpr MetaVariableRef() throws ParseException:
{
- MetaVariableExpr metaVarExp = new MetaVariableExpr();
- VarIdentifier var = new VarIdentifier();
}
{
- <METAVARIABLE>
+ <METAVARIABLE>
{
- metaVarExp.setVar(var);
- var.setValue(token.image);
- return metaVarExp;
+ VarIdentifier var = new VarIdentifier(token.image);
+ return new MetaVariableExpr(var);
}
}
diff --git
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java
index aed358e..60471f1 100644
---
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java
+++
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java
@@ -557,11 +557,20 @@ public class QueryTranslator extends
AbstractLangTranslator implements IStatemen
SourceLocation sourceLoc = dd.getSourceLocation();
String dataverseName = getActiveDataverse(dd.getDataverse());
String datasetName = dd.getName().getValue();
+ String datasetFullyQualifiedName = dataverseName + "." + datasetName;
DatasetType dsType = dd.getDatasetType();
String itemTypeDataverseName =
getActiveDataverse(dd.getItemTypeDataverse());
String itemTypeName = dd.getItemTypeName().getValue();
- String metaItemTypeDataverseName =
getActiveDataverse(dd.getMetaItemTypeDataverse());
- String metaItemTypeName = dd.getMetaItemTypeName().getValue();
+ String itemTypeFullyQualifiedName = itemTypeDataverseName + "." +
itemTypeName;
+ String metaItemTypeDataverseName = null;
+ String metaItemTypeName = null;
+ String metaItemTypeFullyQualifiedName = null;
+ Identifier metaItemTypeId = dd.getMetaItemTypeName();
+ if (metaItemTypeId != null) {
+ metaItemTypeName = metaItemTypeId.getValue();
+ metaItemTypeDataverseName =
getActiveDataverse(dd.getMetaItemTypeDataverse());
+ metaItemTypeFullyQualifiedName = metaItemTypeDataverseName + "." +
metaItemTypeName;
+ }
Identifier ngNameId = dd.getNodegroupName();
String nodegroupName = ngNameId == null ? null : ngNameId.getValue();
String compactionPolicy = dd.getCompactionPolicy();
@@ -573,12 +582,12 @@ public class QueryTranslator extends
AbstractLangTranslator implements IStatemen
boolean bActiveTxn = true;
metadataProvider.setMetadataTxnContext(mdTxnCtx);
MetadataLockUtil.createDatasetBegin(lockManager,
metadataProvider.getLocks(), dataverseName,
- itemTypeDataverseName, itemTypeDataverseName + "." +
itemTypeName, metaItemTypeDataverseName,
- metaItemTypeDataverseName + "." + metaItemTypeName,
nodegroupName, compactionPolicy,
- dataverseName + "." + datasetName, defaultCompactionPolicy);
+ itemTypeDataverseName, itemTypeFullyQualifiedName,
metaItemTypeDataverseName,
+ metaItemTypeFullyQualifiedName, nodegroupName,
compactionPolicy, datasetFullyQualifiedName,
+ defaultCompactionPolicy);
Dataset dataset = null;
try {
- IDatasetDetails datasetDetails = null;
+ IDatasetDetails datasetDetails;
Dataset ds = metadataProvider.findDataset(dataverseName,
datasetName);
if (ds != null) {
if (dd.getIfNotExists()) {
@@ -1372,8 +1381,8 @@ public class QueryTranslator extends
AbstractLangTranslator implements IStatemen
}
}
- if (activeDataverse != null && activeDataverse.getDataverseName()
== dataverseName) {
- activeDataverse = null;
+ if (activeDataverse.getDataverseName().equals(dataverseName)) {
+ activeDataverse = MetadataBuiltinEntities.DEFAULT_DATAVERSE;
}
MetadataManager.INSTANCE.commitTransaction(mdTxnCtx);
return true;
@@ -1383,8 +1392,8 @@ public class QueryTranslator extends
AbstractLangTranslator implements IStatemen
}
if (progress == ProgressState.ADDED_PENDINGOP_RECORD_TO_METADATA) {
- if (activeDataverse != null &&
activeDataverse.getDataverseName() == dataverseName) {
- activeDataverse = null;
+ if (activeDataverse.getDataverseName().equals(dataverseName)) {
+ activeDataverse =
MetadataBuiltinEntities.DEFAULT_DATAVERSE;
}
// #. execute compensation operations
diff --git
a/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/clause/ForClause.java
b/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/clause/ForClause.java
index 9bae2a0..39ad04f 100644
---
a/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/clause/ForClause.java
+++
b/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/clause/ForClause.java
@@ -31,17 +31,14 @@ public class ForClause extends AbstractClause {
private Expression inExpr = null;
public ForClause() {
- super();
}
public ForClause(VariableExpr varExpr, Expression inExpr) {
- super();
this.varExpr = varExpr;
this.inExpr = inExpr;
}
public ForClause(VariableExpr varExpr, Expression inExpr, VariableExpr
posExpr) {
- super();
this.varExpr = varExpr;
this.inExpr = inExpr;
this.posExpr = posExpr;
diff --git
a/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/clause/MetaVariableClause.java
b/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/clause/MetaVariableClause.java
index 7712cf9..bbf7269 100644
---
a/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/clause/MetaVariableClause.java
+++
b/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/clause/MetaVariableClause.java
@@ -27,6 +27,13 @@ import
org.apache.asterix.lang.common.visitor.base.ILangVisitor;
public class MetaVariableClause extends AbstractClause {
private VarIdentifier var;
+ public MetaVariableClause() {
+ }
+
+ public MetaVariableClause(VarIdentifier var) {
+ this.var = var;
+ }
+
@Override
public <R, T> R accept(ILangVisitor<R, T> visitor, T arg) throws
CompilationException {
return ((IAQLPlusVisitor<R, T>) visitor).visitMetaVariableClause(this,
arg);
diff --git
a/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/expression/MetaVariableExpr.java
b/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/expression/MetaVariableExpr.java
index 2964eff..fb03312 100644
---
a/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/expression/MetaVariableExpr.java
+++
b/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/expression/MetaVariableExpr.java
@@ -21,10 +21,15 @@ package org.apache.asterix.lang.aql.expression;
import org.apache.asterix.common.exceptions.CompilationException;
import org.apache.asterix.lang.aql.visitor.base.IAQLPlusVisitor;
import org.apache.asterix.lang.common.expression.VariableExpr;
+import org.apache.asterix.lang.common.struct.VarIdentifier;
import org.apache.asterix.lang.common.visitor.base.ILangVisitor;
public class MetaVariableExpr extends VariableExpr {
+ public MetaVariableExpr(VarIdentifier var) {
+ super(var);
+ }
+
@Override
public <R, T> R accept(ILangVisitor<R, T> visitor, T arg) throws
CompilationException {
return ((IAQLPlusVisitor<R, T>) visitor).visitMetaVariableExpr(this,
arg);
diff --git a/asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj
b/asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj
index 3bb70c1..810d8e9 100644
--- a/asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj
+++ b/asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj
@@ -152,6 +152,7 @@ import org.apache.asterix.lang.common.struct.QuantifiedPair;
import org.apache.asterix.lang.common.struct.VarIdentifier;
import org.apache.asterix.lang.common.util.RangeMapBuilder;
import org.apache.asterix.metadata.utils.MetadataConstants;
+import org.apache.asterix.om.types.BuiltinType;
import org.apache.hyracks.algebricks.common.utils.Pair;
import org.apache.hyracks.algebricks.common.utils.Triple;
import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException;
@@ -184,6 +185,8 @@ class AQLParser extends ScopeChecker implements IParser {
// data generator hints
private static final String DGEN_HINT = "dgen";
+ private static final String INT_TYPE_NAME = "int";
+
private static class IndexParams {
public IndexType type;
public int gramLength;
@@ -797,15 +800,13 @@ List<VarIdentifier> ParameterList() throws ParseException:
{
<LEFTPAREN> (<VARIABLE>
{
- var = new VarIdentifier();
- var.setValue(token.image);
+ var = new VarIdentifier(token.image);
paramList.add(var);
getCurrentScope().addNewVarSymbolToScope(var);
}
( <COMMA> <VARIABLE>
{
- var = new VarIdentifier();
- var.setValue(token.image);
+ var = new VarIdentifier(token.image);
paramList.add(var);
getCurrentScope().addNewVarSymbolToScope(var);
}
@@ -1386,8 +1387,8 @@ TypeReferenceExpression TypeReference() throws
ParseException:
{
id = QualifiedName()
{
- if (id.first == null && id.second.getValue().equalsIgnoreCase("int")) {
- id.second.setValue("int64");
+ if (id.first == null &&
id.second.getValue().equalsIgnoreCase(INT_TYPE_NAME)) {
+ id.second = new Identifier(BuiltinType.AINT64.getTypeName());
}
return new TypeReferenceExpression(id);
@@ -1460,8 +1461,8 @@ FunctionName FunctionName() throws ParseException:
result.function = third;
}
- if (result.function.equalsIgnoreCase("int")) {
- result.function = "int64";
+ if (result.function.equalsIgnoreCase(INT_TYPE_NAME)) {
+ result.function = BuiltinType.AINT64.getTypeName();
}
return result;
}
@@ -2086,8 +2087,6 @@ Expression Literal() throws ParseException:
VariableExpr VariableRef() throws ParseException:
{
- VariableExpr varExp = new VariableExpr();
- VarIdentifier var = new VarIdentifier();
}
{
<VARIABLE>
@@ -2097,13 +2096,13 @@ VariableExpr VariableRef() throws ParseException:
if (isInForbiddenScopes(varName)) {
throw new ParseException("Inside limit clauses, it is disallowed to
reference a variable having the same name as any variable bound in the same
scope as the limit clause.");
}
+ VariableExpr varExp;
if(ident != null) { // exist such ident
+ varExp = new VariableExpr((VarIdentifier)ident);
varExp.setIsNewVar(false);
- varExp.setVar((VarIdentifier)ident);
} else {
- varExp.setVar(var);
+ varExp = new VariableExpr(new VarIdentifier(varName));
}
- var.setValue(varName);
return varExp;
}
}
@@ -2111,18 +2110,16 @@ VariableExpr VariableRef() throws ParseException:
VariableExpr Variable() throws ParseException:
{
- VariableExpr varExp = new VariableExpr();
- VarIdentifier var = new VarIdentifier();
}
{
<VARIABLE>
{
- Identifier ident = lookupSymbol(token.image);
+ String varName = token.image;
+ Identifier ident = lookupSymbol(varName);
+ VariableExpr varExp = new VariableExpr(new VarIdentifier(varName));
if(ident != null) { // exist such ident
varExp.setIsNewVar(false);
}
- varExp.setVar(var);
- var.setValue(token.image);
return varExp;
}
}
diff --git
a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/VariableExpr.java
b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/VariableExpr.java
index 17d0d2f..3ed3221 100644
---
a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/VariableExpr.java
+++
b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/VariableExpr.java
@@ -29,11 +29,6 @@ public class VariableExpr extends AbstractExpression {
private VarIdentifier var;
private boolean isNewVar;
- public VariableExpr() {
- super();
- isNewVar = true;
- }
-
public VariableExpr(VarIdentifier var) {
super();
this.var = var;
diff --git
a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/DatasetDecl.java
b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/DatasetDecl.java
index 0a17b24..ac019fc 100644
---
a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/DatasetDecl.java
+++
b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/DatasetDecl.java
@@ -101,16 +101,12 @@ public class DatasetDecl extends AbstractStatement {
}
}
- public Identifier getMetaName() {
- return name;
- }
-
public Identifier getMetaItemTypeName() {
- return metaItemTypeName == null ? new Identifier() : metaItemTypeName;
+ return metaItemTypeName;
}
public Identifier getMetaItemTypeDataverse() {
- return metaItemTypeDataverse == null ? new Identifier() :
metaItemTypeDataverse;
+ return metaItemTypeDataverse;
}
public String getQualifiedMetaTypeName() {
diff --git
a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/struct/Identifier.java
b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/struct/Identifier.java
index 1443833..7f8171c 100644
---
a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/struct/Identifier.java
+++
b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/struct/Identifier.java
@@ -21,11 +21,8 @@ package org.apache.asterix.lang.common.struct;
import java.util.Objects;
public class Identifier {
- protected String value;
- public Identifier() {
- // default constructor.
- }
+ protected final String value;
public Identifier(String value) {
this.value = value;
@@ -35,10 +32,6 @@ public class Identifier {
return value;
}
- public final void setValue(String value) {
- this.value = value;
- }
-
@Override
public String toString() {
return value;
diff --git
a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/struct/VarIdentifier.java
b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/struct/VarIdentifier.java
index 587dd1c..4fea88b 100644
---
a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/struct/VarIdentifier.java
+++
b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/struct/VarIdentifier.java
@@ -21,10 +21,8 @@ package org.apache.asterix.lang.common.struct;
import java.util.Objects;
public final class VarIdentifier extends Identifier {
- private int id = 0;
- public VarIdentifier() {
- }
+ private int id;
public VarIdentifier(VarIdentifier v) {
this(v.getValue(), v.getId());
@@ -35,7 +33,7 @@ public final class VarIdentifier extends Identifier {
}
public VarIdentifier(String value, int id) {
- this.value = value;
+ super(value);
this.id = id;
}
@@ -48,11 +46,6 @@ public final class VarIdentifier extends Identifier {
}
@Override
- public VarIdentifier clone() {
- return new VarIdentifier(value, id);
- }
-
- @Override
public int hashCode() {
return Objects.hash(value);
}
diff --git
a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/ExpressionToVariableUtil.java
b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/ExpressionToVariableUtil.java
index d8534be..21ae883 100644
---
a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/ExpressionToVariableUtil.java
+++
b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/ExpressionToVariableUtil.java
@@ -96,8 +96,7 @@ public class ExpressionToVariableUtil {
try {
String varName = getGeneratedIdentifier(expr);
VarIdentifier var = new VarIdentifier(varName);
- VariableExpr varExpr = new VariableExpr();
- varExpr.setVar(var);
+ VariableExpr varExpr = new VariableExpr(var);
varExpr.setSourceLocation(expr.getSourceLocation());
return varExpr;
} catch (ParseException e) {
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
index 60cdf2e..e0ad033 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
+++ b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
@@ -181,6 +181,7 @@ import
org.apache.asterix.lang.sqlpp.util.ExpressionToVariableUtil;
import org.apache.asterix.lang.sqlpp.util.FunctionMapUtil;
import org.apache.asterix.lang.sqlpp.util.SqlppVariableUtil;
import org.apache.asterix.om.functions.BuiltinFunctions;
+import org.apache.asterix.om.types.BuiltinType;
import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.hyracks.algebricks.common.utils.Pair;
@@ -216,6 +217,8 @@ class SQLPPParser extends ScopeChecker implements IParser {
private static final String TIES = "TIES";
private static final String UNBOUNDED = "UNBOUNDED";
+ private static final String INT_TYPE_NAME = "int";
+
// error configuration
protected static final boolean REPORT_EXPECTED_TOKENS = false;
@@ -1644,8 +1647,8 @@ TypeReferenceExpression TypeReference() throws
ParseException:
{
id = QualifiedName()
{
- if (id.first == null && id.second.getValue().equalsIgnoreCase("int")) {
- id.second.setValue("int64");
+ if (id.first == null &&
id.second.getValue().equalsIgnoreCase(INT_TYPE_NAME)) {
+ id.second = new Identifier(BuiltinType.AINT64.getTypeName());
}
TypeReferenceExpression typeRef = new TypeReferenceExpression(id);
@@ -1727,8 +1730,8 @@ FunctionName FunctionName() throws ParseException:
result.function = third;
}
- if (result.function.equalsIgnoreCase("int")) {
- result.function = "int64";
+ if (result.function.equalsIgnoreCase(INT_TYPE_NAME)) {
+ result.function = BuiltinType.AINT64.getTypeName();
}
return result;
}
@@ -2564,7 +2567,6 @@ Expression Literal() throws ParseException:
VariableExpr VariableRef() throws ParseException:
{
- VarIdentifier var = new VarIdentifier();
String id = null;
}
{
@@ -2576,13 +2578,12 @@ VariableExpr VariableRef() throws ParseException:
throw new SqlppParseException(getSourceLocation(token),
"Inside limit clauses, it is disallowed to reference a variable having
the same name as any variable bound in the same scope as the limit clause.");
}
- VariableExpr varExp = new VariableExpr();
+ VariableExpr varExp;
if (ident != null) { // exist such ident
- varExp.setVar((VarIdentifier)ident);
+ varExp = new VariableExpr((VarIdentifier)ident);
} else {
- varExp.setVar(var);
+ varExp = new VariableExpr(new VarIdentifier(id));
varExp.setIsNewVar(false);
- var.setValue(id);
}
return addSourceLocation(varExp, token);
}
@@ -2590,7 +2591,6 @@ VariableExpr VariableRef() throws ParseException:
VariableExpr Variable() throws ParseException:
{
- VarIdentifier var = new VarIdentifier();
String id = null;
}
{
@@ -2598,12 +2598,10 @@ VariableExpr Variable() throws ParseException:
{
id = SqlppVariableUtil.toInternalVariableName(id); // prefix user-defined
variables with "$".
Identifier ident = lookupSymbol(id);
- VariableExpr varExp = new VariableExpr();
- if(ident != null) { // exist such ident
+ VariableExpr varExp = new VariableExpr(new VarIdentifier(id));
+ if (ident != null) { // exist such ident
varExp.setIsNewVar(false);
}
- varExp.setVar(var);
- var.setValue(id);
return addSourceLocation(varExp, token);
}
}
