orientechnologies / orientdb

OrientDB is the most versatile DBMS supporting Graph, Document, Reactive, Full-Text and Geospatial models in one Multi-Model product. OrientDB can run distributed (Multi-Master), supports SQL, ACID Transactions, Full-Text indexing and Reactive Queries.
https://orientdb.dev
Apache License 2.0
4.72k stars 869 forks source link

ClassCastException in com.orientechnologies.orient.core.sql.executor.OResultInternal#getProperty Method Due to Direct Casting of OVertexDelegate Instead of Using getRecord() #10171

Closed mikhalov closed 4 months ago

mikhalov commented 4 months ago

OrientDB Version: 3.2.28

Java Version: 17

OS: Windows

Expected behavior

The com.orientechnologies.orient.core.sql.executor.OResultInternal#getProperty method should safely and correctly retrieve properties from an OrientDB document or vertex without causing a ClassCastException. The previous implementation handled this correctly.

Actual behavior

After updating the method, a ClassCastException is thrown when attempting to retrieve properties from an OVertexDelegate instance. The exception message is: java.lang.ClassCastException: class com.orientechnologies.orient.core.record.impl.OVertexDelegate cannot be cast to class com.orientechnologies.orient.core.record.impl.ODocument (com.orientechnologies.orient.core.record.impl.OVertexDelegate and com.orientechnologies.orient.core.record.impl.ODocument are in unnamed module of loader 'app').

Steps to reproduce

  1. Execute a query using select from (traverse from @rid) order by property.
  2. The exception occurs specifically at the moment of executing the order by clause after traversing.

Key Change Highlighted:

Previous getProperty method implementation(3.2.25 version):

public <T> T getProperty(String name) {
    loadElement();
    T result = null;
    if (content != null && content.containsKey(name)) {
      result = (T) wrap(content.get(name));
    } else if (element != null) {
      result = (T) wrap(((ODocument) element.getRecord()).rawField(name));
    }
    if (result instanceof OIdentifiable && ((OIdentifiable) result).getIdentity().isPersistent()) {
      result = (T) ((OIdentifiable) result).getIdentity();
    }
    return result;
}

Updated getProperty method implementation:

public <T> T getProperty(String name) {
    loadElement();
    T result = null;
    if (content != null && content.containsKey(name)) {
      result = (T) wrap(content.get(name));
    } else if (isElement()) {
      result = (T) wrap((T) ODocumentInternal.rawPropertyRead((OElement) element, name));
    }
    if (result instanceof OIdentifiable && ((OIdentifiable) result).getIdentity().isPersistent()) {
      result = (T) ((OIdentifiable) result).getIdentity();
    }
    return result;
}

Additional rawPropertyRead method:

public static <RET> RET rawPropertyRead(OElement element, String propertyName) {
    if (element == null) {
      return null;
    }
    return ((ODocument) element).rawField(propertyName);
}
tglman commented 4 months ago

Hi,

Thanks will check that change/revert back the fix, it was done to avoid to load records only when it was needed, the getRecord do a early loading that may slow down some use cases.

Do you have a stack trace of the case (only the OrientDB parts are needed) ? it will help me to identify the path that has the issue.

Regards

mikhalov commented 4 months ago

java.lang.ClassCastException: class com.orientechnologies.orient.core.record.impl.OVertexDelegate cannot be cast to class com.orientechnologies.orient.core.record.impl.ODocument (com.orientechnologies.orient.core.record.impl.OVertexDelegate and com.orientechnologies.orient.core.record.impl.ODocument are in unnamed module of loader 'app')

at com.orientechnologies.orient.core.record.impl.ODocumentInternal.rawPropertyRead(ODocumentInternal.java:128)
at com.orientechnologies.orient.core.sql.executor.OResultInternal.getProperty(OResultInternal.java:121)
at com.orientechnologies.orient.core.sql.executor.OTraverseResult.getProperty(OTraverseResult.java:20)
at com.orientechnologies.orient.core.sql.parser.OOrderByItem.compare(OOrderByItem.java:96)
at com.orientechnologies.orient.core.sql.parser.OOrderBy.compare(OOrderBy.java:66)
at com.orientechnologies.orient.core.sql.executor.OrderByStep.lambda$init$2(OrderByStep.java:91)
at java.base/java.util.TimSort.countRunAndMakeAscending(TimSort.java:355)
at java.base/java.util.TimSort.sort(TimSort.java:220)
at java.base/java.util.Arrays.sort(Arrays.java:1307)
at java.base/java.util.ArrayList.sort(ArrayList.java:1721)
at com.orientechnologies.orient.core.sql.executor.OrderByStep.init(OrderByStep.java:91)
at com.orientechnologies.orient.core.sql.executor.OrderByStep.internalStart(OrderByStep.java:43)
at com.orientechnologies.orient.core.sql.executor.AbstractExecutionStep.start(AbstractExecutionStep.java:79)
at com.orientechnologies.orient.core.sql.executor.OSelectExecutionPlan.start(OSelectExecutionPlan.java:41)
at com.orientechnologies.orient.core.sql.parser.OLocalResultSet.start(OLocalResultSet.java:36)
at com.orientechnologies.orient.core.sql.parser.OLocalResultSet.<init>(OLocalResultSet.java:27)
at com.orientechnologies.orient.core.sql.parser.OSelectStatement.execute(OSelectStatement.java:357)
at com.orientechnologies.orient.core.sql.parser.OStatement.execute(OStatement.java:76)
at com.orientechnologies.orient.core.db.document.ODatabaseDocumentEmbedded.query(ODatabaseDocumentEmbedded.java:592)
mikhalov commented 4 months ago

Thank you for your prompt response. I understand the intent behind the change to optimize performance by delaying record loading. However, after updating the project to the latest version of OrientDB, I noticed that many of the tests started failing due to this ClassCastException.

To isolate the issue, I created a fork of the project and modified the rawPropertyRead method as follows to revert to the earlier behavior:

public static <RET> RET rawPropertyRead(OElement element, String propertyName) {
    if (element == null) {
        return null;
    }
    return ((ODocument) element.getRecord()).rawField(propertyName);
}

After compiling this version and replacing the main dependency in pom.xml with the modified version, tests no longer fail. This adjustment seems to resolve the ClassCastException issue I encountered.

I appreciate your help in looking into this matter and am happy to provide any further information you may need.

Regards

tglman commented 4 months ago

Hi,

Yep I've done a fix that is pretty much the same fix, will be release 3.2.29

Regards

mikhalov commented 4 months ago

Great to hear that a fix has been implemented and will be included in release 3.2.29. I appreciate the quick response and action taken on this issue. Looking forward to the update!