Skip to content

Commit

Permalink
Fixed issue #1056: new "return" keyword on UPDATE and DELETE
Browse files Browse the repository at this point in the history
  • Loading branch information
@lvca
lvca committed Feb 2, 2014
1 parent 418a949 commit f9a136c
Show file tree
Hide file tree
Showing 5 changed files with 251 additions and 137 deletions.
30 changes: 30 additions & 0 deletions core/src/main/java/com/orientechnologies/orient/core/sql/OCommandExecutorSQLAbstract.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public abstract class OCommandExecutorSQLAbstract extends OCommandExecutorAbstra
public static final String KEYWORD_SKIP = "SKIP";
public static final String KEYWORD_OFFSET = "OFFSET";
public static final String KEYWORD_TIMEOUT = "TIMEOUT";
public static final String KEYWORD_LOCK = "LOCK";
public static final String KEYWORD_RETURN = "RETURN";
public static final String KEYWORD_KEY = "key";
public static final String KEYWORD_RID = "rid";
public static final String CLUSTER_PREFIX = "CLUSTER:";
Expand Down Expand Up @@ -100,4 +102,32 @@ else if (word.equals(TIMEOUT_STRATEGY.RETURN.toString()))

return true;
}

/**
* Parses the lock keyword if found.
*/
protected String parseLock() throws OCommandSQLParsingException {
parserNextWord(true);
final String lockStrategy = parserGetLastWord();

if (!lockStrategy.equalsIgnoreCase("NONE") && !lockStrategy.equalsIgnoreCase("RECORD"))
throwParsingException("Invalid " + KEYWORD_LOCK + " value set to '" + lockStrategy
+ "' but it should be NONE (default) or RECORD. Example: " + KEYWORD_LOCK + " RECORD");

return lockStrategy;
}

/**
* Parses the returning keyword if found.
*/
protected String parseReturn() throws OCommandSQLParsingException {
parserNextWord(true);
final String returning = parserGetLastWord();

if (!returning.equalsIgnoreCase("COUNT") && !returning.equalsIgnoreCase("BEFORE") && !returning.equalsIgnoreCase("AFTER"))
throwParsingException("Invalid " + KEYWORD_RETURN + " value set to '" + returning
+ "' but it should be COUNT (default), BEFORE or AFTER. Example: " + KEYWORD_RETURN + " BEFORE");

return returning;
}
}
114 changes: 92 additions & 22 deletions core/src/main/java/com/orientechnologies/orient/core/sql/OCommandExecutorSQLDelete.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@
*/
package com.orientechnologies.orient.core.sql;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import com.orientechnologies.common.collection.OCompositeKey;
import com.orientechnologies.common.parser.OStringParser;
import com.orientechnologies.orient.core.command.OCommandDistributedReplicateRequest;
Expand All @@ -31,12 +27,20 @@
import com.orientechnologies.orient.core.index.OCompositeIndexDefinition;
import com.orientechnologies.orient.core.index.OIndex;
import com.orientechnologies.orient.core.index.OIndexDefinition;
import com.orientechnologies.orient.core.record.ORecord;
import com.orientechnologies.orient.core.record.ORecordAbstract;
import com.orientechnologies.orient.core.record.impl.ODocument;
import com.orientechnologies.orient.core.sql.filter.OSQLFilter;
import com.orientechnologies.orient.core.sql.filter.OSQLFilterCondition;
import com.orientechnologies.orient.core.sql.query.OSQLAsynchQuery;
import com.orientechnologies.orient.core.sql.query.OSQLQuery;
import com.orientechnologies.orient.core.storage.OStorage;
import com.orientechnologies.orient.core.storage.OStorageEmbedded;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

/**
* SQL UPDATE command.
Expand All @@ -53,6 +57,9 @@ public class OCommandExecutorSQLDelete extends OCommandExecutorSQLAbstract imple
private OSQLQuery<ODocument> query;
private String indexName = null;
private int recordCount = 0;
private String lockStrategy = "NONE";
private String returning = "COUNT";
private List<ORecord<?>> allDeletedRecords;

private OSQLFilter compiledFilter;

Expand Down Expand Up @@ -80,11 +87,20 @@ public OCommandExecutorSQLDelete parse(final OCommandRequest iRequest) {
indexName = subjectName.substring(OCommandExecutorSQLAbstract.INDEX_PREFIX.length());

if (!parserIsEnded()) {
parserNextWord(true);
while (!parserIsEnded()) {
final String word = parserGetLastWord();

if (word.equals(KEYWORD_LOCK))
lockStrategy = parseLock();
else if (word.equals(KEYWORD_RETURN))
returning = parseReturn();
else if (word.equalsIgnoreCase(KEYWORD_WHERE))
compiledFilter = OSQLEngine.getInstance().parseCondition(parserText.substring(parserGetCurrentPosition()),
getContext(), KEYWORD_WHERE);

parserNextWord(true);
}

if (parserGetLastWord().equalsIgnoreCase(KEYWORD_WHERE))
compiledFilter = OSQLEngine.getInstance().parseCondition(parserText.substring(parserGetCurrentPosition()), getContext(),
KEYWORD_WHERE);
} else
parserSetCurrentPosition(-1);

Expand All @@ -93,6 +109,23 @@ public OCommandExecutorSQLDelete parse(final OCommandRequest iRequest) {
query = database.command(new OSQLAsynchQuery<ODocument>(subjectName.substring(1, subjectName.length() - 1), this));

} else {
parserNextWord(true);

while (!parserIsEnded()) {
final String word = parserGetLastWord();

if (word.equals(KEYWORD_LOCK))
lockStrategy = parseLock();
else if (word.equals(KEYWORD_RETURN))
returning = parseReturn();
else {
parserGoBack();
break;
}

parserNextWord(true);
}

final String condition = parserGetCurrentPosition() > -1 ? " " + parserText.substring(parserGetCurrentPosition()) : "";
query = database.command(new OSQLAsynchQuery<ODocument>("select from " + subjectName + condition, this));
}
Expand All @@ -104,13 +137,25 @@ public Object execute(final Map<Object, Object> iArgs) {
if (query == null && indexName == null)
throw new OCommandExecutionException("Cannot execute the command because it has not been parsed yet");

if (!returning.equalsIgnoreCase("COUNT"))
allDeletedRecords = new ArrayList<ORecord<?>>();

if (query != null) {
// AGAINST CLUSTERS AND CLASSES
if (lockStrategy.equals("RECORD"))
query.getContext().setVariable("$locking", OStorage.LOCKING_STRATEGY.KEEP_EXCLUSIVE_LOCK);

query.execute(iArgs);
return recordCount;

if (returning.equalsIgnoreCase("COUNT"))
// RETURNS ONLY THE COUNT
return recordCount;
else
// RETURNS ALL THE DELETED RECORDS
return allDeletedRecords;

} else {
// AGAINST INDEXES

if (compiledFilter != null)
compiledFilter.bindParameters(iArgs);

Expand All @@ -122,9 +167,22 @@ public Object execute(final Map<Object, Object> iArgs) {
Object value = VALUE_NOT_FOUND;

if (compiledFilter == null || compiledFilter.getRootCondition() == null) {
final long total = index.getSize();
index.clear();
return total;
if (returning.equalsIgnoreCase("COUNT")) {
// RETURNS ONLY THE COUNT
final long total = index.getSize();
index.clear();
return total;
} else {
// RETURNS ALL THE DELETED RECORDS
for (Iterator<OIdentifiable> it = index.valuesIterator(); it.hasNext();) {
OIdentifiable rec = it.next();
if (rec != null)
rec = rec.getRecord();
allDeletedRecords.add((ORecord<?>) rec);
}
return allDeletedRecords;
}

} else {
if (KEYWORD_KEY.equalsIgnoreCase(compiledFilter.getRootCondition().getLeft().toString()))
// FOUND KEY ONLY
Expand Down Expand Up @@ -153,7 +211,11 @@ else if (KEYWORD_RID.equalsIgnoreCase(compiledFilter.getRootCondition().getLeft(
} else
result = index.remove(key);

return result ? 1 : 0;
if (returning.equalsIgnoreCase("COUNT"))
return result ? 1 : 0;
else
// TODO: REFACTOR INDEX TO RETURN DELETED ITEMS
throw new UnsupportedOperationException();
}
}
}
Expand All @@ -164,22 +226,30 @@ else if (KEYWORD_RID.equalsIgnoreCase(compiledFilter.getRootCondition().getLeft(
public boolean result(final Object iRecord) {
final ORecordAbstract<?> record = (ORecordAbstract<?>) iRecord;

if (record.getIdentity().isValid()) {
// RESET VERSION TO DISABLE MVCC AVOIDING THE CONCURRENT EXCEPTION IF LOCAL CACHE IS NOT UPDATED
record.getRecordVersion().disable();
record.delete();
recordCount++;
return true;
try {
if (record.getIdentity().isValid()) {
if (returning.equalsIgnoreCase("BEFORE"))
allDeletedRecords.add(record);

// RESET VERSION TO DISABLE MVCC AVOIDING THE CONCURRENT EXCEPTION IF LOCAL CACHE IS NOT UPDATED
record.getRecordVersion().disable();
record.delete();
recordCount++;
return true;
}
return false;
} finally {
if (lockStrategy.equalsIgnoreCase("RECORD"))
((OStorageEmbedded) getDatabase().getStorage()).releaseWriteLock(record.getIdentity());
}
return false;
}

public boolean isReplicated() {
return indexName != null;
}

public String getSyntax() {
return "DELETE FROM <Class>|RID|cluster:<cluster> [WHERE <condition>*]";
return "DELETE FROM <Class>|RID|cluster:<cluster> [LOCK <NONE|RECORD>] [RETURNING <COUNT|BEFORE>] [WHERE <condition>*]";

This comment has been minimized.

Sign in to view

Copy link
@aruld

aruld Feb 10, 2014

Another spot you missed the renaming form RETURNING -> RETURN.
}

private Object getIndexKey(final OIndexDefinition indexDefinition, Object value) {
Expand Down
Loading

3 comments on commit f9a136c

@kowalot
Copy link
Contributor

@kowalot kowalot commented on f9a136c Feb 2, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test command in console generates following exception:
orientdb {test}> update #54:0 SET Age=4 RETURN BEFORE
Updated
orientdb {test}>
Error: java.util.IllegalFormatConversionException: d != java.util.ArrayList

@kowalot
Copy link
Contributor

@kowalot kowalot commented on f9a136c Feb 2, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next problem
orientdb> connect remote:localhost/test root root
Connecting to database [remote:localhost/test] with user 'root'...OK
orientdb {test}> reload record #54:0

ODocument - Class: Actor id: #54:0 v.10

       FirstName : Robert              
        LastName : Pattinson           
             Age : 1

OK
orientdb {test}> UPDATE #54:0 SET Age=Age+1

Updated 1 record(s) in 0,001000 sec(s).

orientdb {test}> reload record #54:0

ODocument - Class: Actor id: #54:0 v.11

       FirstName : Robert              
        LastName : Pattinson           
             Age : null

OK
orientdb {test}>

@lvca
Copy link
Member Author

@lvca lvca commented on f9a136c Feb 2, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed the console bug. About the update we don't support that, but rather:

UPDATE #9:50 SET Age=eval( 'Age + 1' ) return after

Set the Age = something before to increment.

Please sign in to comment.