Skip to content

Commit

Permalink
fix: throw a parsing exception when reserved names are used for varia…
Browse files Browse the repository at this point in the history
…bles in sqlscripts

Fixed issue #1489
  • Loading branch information
lvca committed Feb 29, 2024
1 parent 2b0f8d9 commit c85a47b
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 16 deletions.
2 changes: 1 addition & 1 deletion engine/src/main/grammar/SQLGrammar.jjt
Original file line number Diff line number Diff line change
Expand Up @@ -4044,7 +4044,7 @@ LetStatement LetStatement():
{ }
{
<LET>
jjtThis.name = Identifier()
jjtThis.variableName = Identifier()
<EQ>
(
LOOKAHEAD(Statement())
Expand Down
19 changes: 16 additions & 3 deletions engine/src/main/java/com/arcadedb/query/sql/SQLQueryEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.arcadedb.database.DatabaseInternal;
import com.arcadedb.database.Identifiable;
import com.arcadedb.exception.CommandExecutionException;
import com.arcadedb.exception.CommandSQLParsingException;
import com.arcadedb.function.FunctionDefinition;
import com.arcadedb.query.QueryEngine;
import com.arcadedb.query.sql.executor.BasicCommandContext;
Expand All @@ -44,10 +45,12 @@
import static com.arcadedb.query.sql.parser.SqlParserTreeConstants.JJTLIMIT;

public class SQLQueryEngine implements QueryEngine {
public static final String ENGINE_NAME = "sql";
public static final String ENGINE_NAME = "sql";
protected final DatabaseInternal database;
protected final DefaultSQLFunctionFactory functions;
protected final DefaultSQLMethodFactory methods;
public static final Set<String> RESERVED_VARIABLE_NAMES = Set.of("parent", "current", "depth",
"path", "stack", "history");

public static class SQLQueryEngineFactory implements QueryEngineFactory {
@Override
Expand Down Expand Up @@ -126,7 +129,8 @@ public boolean isDDL() {
};
}

public static Object foreachRecord(final Callable<Object, Identifiable> iCallable, Object iCurrent, final CommandContext iContext) {
public static Object foreachRecord(final Callable<Object, Identifiable> iCallable, Object iCurrent,
final CommandContext iContext) {
if (iCurrent == null)
return null;

Expand Down Expand Up @@ -180,7 +184,8 @@ public SQLFunction getFunction(final String name) {
// WRAP LIBRARY FUNCTION TO SQL FUNCTION TO BE EXECUTED BY SQL ENGINE
sqlFunction = new SQLFunctionAbstract(name) {
@Override
public Object execute(final Object iThis, final Identifiable iCurrentRecord, final Object iCurrentResult, final Object[] iParams,
public Object execute(final Object iThis, final Identifiable iCurrentRecord, final Object iCurrentResult,
final Object[] iParams,
final CommandContext iContext) {
return function.execute(iParams);
}
Expand All @@ -207,4 +212,12 @@ public SQLMethod getMethod(final String name) {
public static Statement parse(final String query, final DatabaseInternal database) {
return database.getStatementCache().get(query);
}

public static void validateVariableName(String varName) {
if (varName.startsWith("$"))
varName = varName.substring(1);

if (SQLQueryEngine.RESERVED_VARIABLE_NAMES.contains(varName))
throw new CommandSQLParsingException(varName + " is a reserved variable");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ private ResultSet executeInternal(final List<Statement> statements, final Comman
}

if (stm instanceof LetStatement)
scriptContext.declareScriptVariable(((LetStatement) stm).getName().getStringValue());
scriptContext.declareScriptVariable(((LetStatement) stm).getVariableName().getStringValue());
}

return new LocalResultSet(plan);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
package com.arcadedb.query.sql.parser;

import com.arcadedb.database.Database;
import com.arcadedb.exception.CommandSQLParsingException;
import com.arcadedb.query.sql.SQLQueryEngine;
import com.arcadedb.query.sql.executor.BasicCommandContext;
import com.arcadedb.query.sql.executor.CommandContext;
import com.arcadedb.query.sql.executor.ForEachExecutionPlan;
Expand Down Expand Up @@ -77,7 +79,9 @@ public UpdateExecutionPlan createExecutionPlan(final CommandContext context, fin
if (nextProg < 0)
FOREACH_VARIABLE_PROGR.set(0);

Identifier varName = new Identifier("$__ARCADEDB_FOREACH_VAR_" + nextProg);
SQLQueryEngine.validateVariableName(loopVariable.getStringValue());

final Identifier varName = new Identifier("$__ARCADEDB_FOREACH_VAR_" + nextProg);
plan.chain(new GlobalLetExpressionStep(varName, loopValues, context, enableProfiling));
plan.chain(new ForEachStep(loopVariable, new Expression(varName), statements, context, enableProfiling));
return plan;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
/* JavaCCOptions:MULTI=true,NODE_USES_PARSER=false,VISITOR=true,TRACK_TOKENS=true,NODE_PREFIX=O,NODE_EXTENDS=,NODE_FACTORY=,SUPPORT_USERTYPE_VISIBILITY_PUBLIC=true */
package com.arcadedb.query.sql.parser;

import com.arcadedb.query.sql.SQLQueryEngine;
import com.arcadedb.query.sql.executor.CommandContext;
import com.arcadedb.query.sql.executor.InternalResultSet;
import com.arcadedb.query.sql.executor.Result;
Expand All @@ -28,7 +29,7 @@
import java.util.*;

public class LetStatement extends SimpleExecStatement {
protected Identifier name;
protected Identifier variableName;
protected Statement statement;
protected Expression expression;

Expand All @@ -38,6 +39,8 @@ public LetStatement(final int id) {

@Override
public ResultSet executeSimple(final CommandContext context) {
SQLQueryEngine.validateVariableName(variableName.getStringValue());

Object result;
if (expression != null) {
result = expression.execute((Result) null, context);
Expand All @@ -55,9 +58,9 @@ public ResultSet executeSimple(final CommandContext context) {

if (context != null) {
if (context.getParent() != null) {
context.getParent().setVariable(name.getStringValue(), result);
context.getParent().setVariable(variableName.getStringValue(), result);
} else {
context.setVariable(name.getStringValue(), result);
context.setVariable(variableName.getStringValue(), result);
}
}
return new InternalResultSet();
Expand All @@ -66,7 +69,7 @@ public ResultSet executeSimple(final CommandContext context) {
@Override
public void toString(final Map<String, Object> params, final StringBuilder builder) {
builder.append("LET ");
name.toString(params, builder);
variableName.toString(params, builder);
builder.append(" = ");
if (statement != null) {
statement.toString(params, builder);
Expand All @@ -78,7 +81,7 @@ public void toString(final Map<String, Object> params, final StringBuilder build
@Override
public LetStatement copy() {
final LetStatement result = new LetStatement(-1);
result.name = name == null ? null : name.copy();
result.variableName = variableName == null ? null : variableName.copy();
result.statement = statement == null ? null : statement.copy();
result.expression = expression == null ? null : expression.copy();
return result;
Expand All @@ -93,7 +96,7 @@ public boolean equals(final Object o) {

final LetStatement that = (LetStatement) o;

if (!Objects.equals(name, that.name))
if (!Objects.equals(variableName, that.variableName))
return false;
if (!Objects.equals(statement, that.statement))
return false;
Expand All @@ -102,14 +105,14 @@ public boolean equals(final Object o) {

@Override
public int hashCode() {
int result = name != null ? name.hashCode() : 0;
int result = variableName != null ? variableName.hashCode() : 0;
result = 31 * result + (statement != null ? statement.hashCode() : 0);
result = 31 * result + (expression != null ? expression.hashCode() : 0);
return result;
}

public Identifier getName() {
return name;
public Identifier getVariableName() {
return variableName;
}
}
/* JavaCC - OriginalChecksum=cc646e5449351ad9ced844f61b687928 (do not edit this line) */
Original file line number Diff line number Diff line change
Expand Up @@ -16077,7 +16077,7 @@ final public LetStatement LetStatement() throws ParseException {/*@bgen(jjtree)
jjtn000.jjtSetFirstToken(getToken(1));
try {
jj_consume_token(LET);
jjtn000.name = Identifier();
jjtn000.variableName = Identifier();
jj_consume_token(EQ);
if (jj_2_145(2147483647)) {
jjtn000.statement = StatementInternal();
Expand Down
17 changes: 17 additions & 0 deletions engine/src/test/java/com/arcadedb/query/sql/BatchTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package com.arcadedb.query.sql;

import com.arcadedb.TestHelper;
import com.arcadedb.exception.CommandSQLParsingException;
import com.arcadedb.query.sql.executor.Result;
import com.arcadedb.query.sql.executor.ResultSet;
import org.junit.jupiter.api.Assertions;
Expand Down Expand Up @@ -149,4 +150,20 @@ public void testForeachWithReturn() {
Assertions.assertFalse(result.hasNext());
}

@Test
public void testUsingReservedVariableNames() {
try {
database.command("sqlscript", "FOREACH ($parent IN [1, 2, 3]){\nRETURN;\n}");
Assertions.fail();
} catch (CommandSQLParsingException e) {
// EXPECTED
}

try {
database.command("sqlscript", "LET parent = 33;");
Assertions.fail();
} catch (CommandSQLParsingException e) {
// EXPECTED
}
}
}

0 comments on commit c85a47b

Please sign in to comment.