[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 <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Dmitry Lychagin <dmitry.lychagin@couchbase.com>
Reviewed-by: Ali Alsuliman <ali.al.solaiman@gmail.com>
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 @@
@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 @@
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 @@
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 @@
}
}
- 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 @@
}
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 @@
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 @@
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 @@
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.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 @@
// 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 @@
{
<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 @@
{
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 @@
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 @@
VariableExpr VariableRef() throws ParseException:
{
- VariableExpr varExp = new VariableExpr();
- VarIdentifier var = new VarIdentifier();
}
{
<VARIABLE>
@@ -2097,13 +2096,13 @@
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 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 @@
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 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 @@
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 @@
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 @@
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 VarIdentifier(String value, int id) {
- this.value = value;
+ super(value);
this.id = id;
}
@@ -48,11 +46,6 @@
}
@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 @@
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.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 @@
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 @@
{
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 @@
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 @@
VariableExpr VariableRef() throws ParseException:
{
- VarIdentifier var = new VarIdentifier();
String id = null;
}
{
@@ -2576,13 +2578,12 @@
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 Variable() throws ParseException:
{
- VarIdentifier var = new VarIdentifier();
String id = null;
}
{
@@ -2598,12 +2598,10 @@
{
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);
}
}
