Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

execute() in transactions doesn't UPDATE records #8632

Closed
mmacfadden opened this issue Oct 27, 2018 · 6 comments
Closed

execute() in transactions doesn't UPDATE records #8632

mmacfadden opened this issue Oct 27, 2018 · 6 comments

Comments

@mmacfadden
Copy link
Contributor

mmacfadden commented Oct 27, 2018

OrientDB Version: 3.0.12

Java Version: Java 8

OS: Linux

Expected behavior

A script run in a transaction should atomically commit or fail.

Actual behavior

The script only partially executes and appears to fail silently.

Steps to reproduce

Execute this Java class with a server running locally.

import java.util.HashMap;

import com.orientechnologies.orient.core.db.ODatabaseSession;
import com.orientechnologies.orient.core.db.ODatabaseType;
import com.orientechnologies.orient.core.db.OrientDB;
import com.orientechnologies.orient.core.db.OrientDBConfig;

public class OdbScriptTest {
  public static final String SQL = "sql";
  public static final String DB_NAME = "test";
  
  public static void main(String[] args) {
    final OrientDB odb = new OrientDB("remote:localhost", "root", "password", OrientDBConfig.defaultConfig());
    odb.create(DB_NAME, ODatabaseType.PLOCAL);
    final ODatabaseSession db = odb.open(DB_NAME, "admin", "admin");

    final String schema = 
        "CREATE CLASS Child;\n" + 
        "CREATE PROPERTY Child.id STRING;\n" +
        "CREATE CLASS Parent;\n" +
        "CREATE PROPERTY Parent.children LINKMAP Child;\n" + 
        "CREATE PROPERTY Parent.id STRING;";

    db.execute(SQL, schema, new HashMap<String, Object>());

    final String data = 
        "INSERT INTO Child SET id = \"child\";\n" +
        "INSERT INTO Parent SET children = {}, id = \"parent\";\n" +
        "LET newChild = SELECT FROM Child WHERE id = \"child\";\n" +
        "UPDATE Parent SET children[\"child\"] = $newChild[0];";

    db.execute(SQL, data, new HashMap<>());

    System.out.println("Child Exists: " + db.command("SELECT FROM Child", new HashMap<>()).next());
    System.out.println("Parent has Child: " + db.command("SELECT FROM Parent", new HashMap<>()).next());

    final String delete = 
        "LET parents = SELECT FROM Parent WHERE id = \"parent\";\n" + 
        "LET p = $parents[0];\n" + 
        "LET directChildrenToDelete = SELECT expand($p.children.`child`);\n" +
        "LET allChildrenToDelete = TRAVERSE children FROM (SELECT expand($directChildrenToDelete));\n" +
        "DELETE FROM (SELECT expand($allChildrenToDelete));\n" +
        "UPDATE (SELECT expand($p)) REMOVE children.`child`;";

    db.begin();
    db.execute(SQL, delete, new HashMap<>());
    db.commit();

    Long childCount = db
        .command("SELECT count(*) as count FROM Child", new HashMap<>())
        .next()
        .toElement()
        .getProperty("count");
    System.out.println("Child deleted: " + childCount);

    System.out.println("Parent not updated: " + db.command("SELECT FROM Parent", new HashMap<String, Object>()).next());

    db.close();
    odb.close();
  }
}

This will print:

Child Exists: Child#25:0{id:child} v1
Parent has Child: Parent#33:0{children:[1],id:parent} v2
Child deleted: 0
Parent not updated: Parent#33:0{children:[1],id:parent} v2

The DELETE FROM (SELECT expand($allChildrenToDelete)); successfully deleted the child record. But the UPDATE (SELECT expand($p)) REMOVE children.child; did not work. Things to note:

  1. If you use an in-memory database the code executes just fine.
  2. If you remove the db.begin() and db.comit() from around the final execute the code works.
  3. If the "delete" script looked only like this UPDATE Parent REMOVE children.child WHERE id = "parent"; It still does not work.
  4. Trying to add BEGIN; and COMMIT; within the script causes an exception.
  5. In our real case there are other commands outside of this script execution that need to exist in the transaction and be committed or rolled back atomically with this script. So the db.begin() and db.commit() are needed to be outside of the script.
@mmacfadden
Copy link
Contributor Author

@luigidellaquila I did check this one, and it looks like it was not fixed by closing #8634, as you suspected.

@luigidellaquila
Copy link
Member

Thank you very much @mmacfadden , I'll check this asap

Thanks

Luigi

@mmacfadden
Copy link
Contributor Author

@luigidellaquila Greetings! I have updated the test case, so that it is easily re-runable. I have also converted the schema and data creation to use the API instead of using scripts, just to rule out errors in the schema or data. I just tested this with the 3.0.12 and verified the issue remains. Here is the updated test case:

import java.util.HashMap;
import java.util.Map;

import com.orientechnologies.orient.core.db.ODatabaseSession;
import com.orientechnologies.orient.core.db.ODatabaseType;
import com.orientechnologies.orient.core.db.OrientDB;
import com.orientechnologies.orient.core.db.OrientDBConfig;
import com.orientechnologies.orient.core.metadata.schema.OClass;
import com.orientechnologies.orient.core.metadata.schema.OSchema;
import com.orientechnologies.orient.core.metadata.schema.OType;
import com.orientechnologies.orient.core.record.OElement;

public class OdbScriptTest2 {
  public static final String SQL = "sql";
  public static final String DB_NAME = "test";
  public static final String CHILD = "Child";
  public static final String PARENT = "Parent";

  public static void main(String[] args) {
    final OrientDB odb = new OrientDB("remote:localhost", "root", "root", OrientDBConfig.defaultConfig());
    if (!odb.exists(DB_NAME)) {
      odb.create(DB_NAME, ODatabaseType.PLOCAL);
    }

    final ODatabaseSession db = odb.open(DB_NAME, "admin", "admin");

    final OSchema schema = db.getMetadata().getSchema();

    if (schema.existsClass(CHILD)) {
      schema.dropClass(CHILD);
    }

    final OClass childClass = schema.createClass("Child");
    childClass.createProperty("id", OType.STRING);

    if (schema.existsClass(PARENT)) {
      schema.dropClass(PARENT);
    }

    final OClass parentClass = schema.createClass("Parent");
    parentClass.createProperty("id", OType.STRING);
    parentClass.createProperty("children", OType.LINKMAP, childClass);

    //
    // Create the Data
    //
    
    // Child
    final OElement childDoc = db.newElement("Child");
    childDoc.setProperty("id", "child");
    childDoc.save();

    // Parent
    final OElement parentDoc = db.newElement("Parent");
    parentDoc.setProperty("id", "parent");
    final Map<String, OElement> children = new HashMap<>();
    children.put("child", childDoc);
    parentDoc.setProperty("children", children);
    parentDoc.save();
    
    // Check The Current Database State

    System.out.println("Child exists: " + db.command("SELECT FROM Child", new HashMap<>()).next());
    System.out.println("Parent has Child: " + db.command("SELECT FROM Parent", new HashMap<>()).next());

    // This script should delete the child, and remove the child from the parent's map.
    final String delete = "LET parents = SELECT FROM Parent WHERE id = \"parent\";\n" 
        + "LET p = $parents[0];\n"
        + "LET directChildrenToDelete = SELECT expand($p.children.`child`);\n"
        + "LET allChildrenToDelete = TRAVERSE children FROM (SELECT expand($directChildrenToDelete));\n"
        + "DELETE FROM (SELECT expand($allChildrenToDelete));\n"
        + "UPDATE (SELECT expand($p)) REMOVE children.`child`;";

    // Update inside a transaction
    db.begin();
    db.execute(SQL, delete, new HashMap<>());
    db.commit();

    // Check the updated state
    final Long childCount = db
        .command("SELECT count(*) as count FROM Child", new HashMap<>())
        .next()
        .toElement()
        .getProperty("count");
    System.out.println("Child deleted. Number of Child documents: " + childCount);

    final OElement updatedParent = db
        .command("SELECT FROM Parent", new HashMap<String, Object>())
        .next()
        .toElement();
    System.out.println("Parent not updated: " + updatedParent);
    
    final Map<String, OElement> updatedChildren = updatedParent.getProperty("children");
    System.out.println("Updated children: " + updatedChildren);
    
    final OElement updatedChild = updatedChildren.get(CHILD);
    System.out.println("Child does not exist: " + updatedChild);
    
    db.close();
    odb.close();
  }
}

@mmacfadden
Copy link
Contributor Author

As a side note, I tried pulling the 3.0.12 tag, and importing into eclipse so that I could help debug, but I seem to be unable to get the code to compile in eclipse or in maven. If I can get it working I am happy to help debug. Is there a stable tag, or a trick to getting it building?

@mmacfadden
Copy link
Contributor Author

mmacfadden commented Jan 25, 2019

@luigidellaquila Finally got to do some debugging. I noticed that if you look at the script execution context here:

{p=Parent#45:0{id:parent,children:[0]} v1, directChildrenToDelete=com.orientechnologies.orient.core.sql.executor.OInternalResultSet@59f18f43, allChildrenToDelete=com.orientechnologies.orient.core.sql.executor.OInternalResultSet@7d5d83ea, parents=com.orientechnologies.orient.core.sql.executor.OInternalResultSet@53f37f4b}

We can see that the Parent element has actually had its child removed from the LINKMAP. So it looks like the execute worked, as far as the script execution.

When committing, in OAbstractPaginatedStorage#commit:

final Collection<ORecordOperation> recordOperations = transaction.getRecordOperations();

We see the following for recordOpertions:

{#33:0=ORecordOperation [record=Child#33:0{id:child} v1, type=DELETE], #45:0=ORecordOperation [record=Parent#45:0{id:parent,children:[0]} v1, type=UPDATE]}

The Parent record has been marked for update. However if we inspect the ORecordOperation for the Parent UPDATE. I see that the record._contentChanged flag is set to false.

A few lines later (2148) we call OAbstractPaginatedStorage.commitEntry. On line 5008 we call doUpdateRecord passing in false for updateContent. Thus on line 4422 we skip the call to cluster.updateRecord.

Therefore it seems like we never write the updated Parent record to storage. The questions is why is the _contentChanged flag in the record set to false?

@mmacfadden
Copy link
Contributor Author

I also noticed that when the com.orientechnologies.orient.core.sql.parser.OUpdateStatement.execute method completes, the parent element's record within the parentCtx variable has is't _contextChanged field set to true. However, somehow by the time we get to the commit, it is set to false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants