This is an automated email from the ASF dual-hosted git repository.

ntimofeev pushed a commit to branch STABLE-4.2
in repository https://gitbox.apache.org/repos/asf/cayenne.git


The following commit(s) were added to refs/heads/STABLE-4.2 by this push:
     new c987980c7 CAY-2841 Multi column ColumnSelect with SHARED_CACHE fails 
after 1st select
c987980c7 is described below

commit c987980c7229782cd813966d6dfc56a67bccea15
Author: Nikita Timofeev <stari...@gmail.com>
AuthorDate: Wed Feb 28 14:33:39 2024 +0400

    CAY-2841 Multi column ColumnSelect with SHARED_CACHE fails after 1st select
---
 RELEASE-NOTES.txt                                  |  1 +
 .../cayenne/access/DataDomainQueryAction.java      | 96 +++++++++++++++++-----
 .../org/apache/cayenne/query/ColumnSelectIT.java   | 19 +++++
 3 files changed, 94 insertions(+), 22 deletions(-)

diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index 6044843b1..dbad3137f 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -22,6 +22,7 @@ CAY-2813 Regression: Constants.CI_PROPERTY flag is no longer 
working for MySQL
 CAY-2815 Incorrect translation of aliased expression
 CAY-2838 Vertical Inheritance: Problem setting db attribute to null via 
flattened path
 CAY-2840 Vertical Inheritance: Missing subclass attributes with joint prefetch
+CAY-2841 Multi column ColumnSelect with SHARED_CACHE fails after 1st select
 
 ----------------------------------
 Release: 4.2
diff --git 
a/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java
 
b/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java
index f95c2df75..2bf31c551 100644
--- 
a/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java
+++ 
b/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java
@@ -54,6 +54,7 @@ import org.apache.cayenne.util.GenericResponse;
 import org.apache.cayenne.util.ListResponse;
 import org.apache.cayenne.util.Util;
 
+import java.lang.reflect.Constructor;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -89,6 +90,7 @@ class DataDomainQueryAction implements QueryRouter, 
OperationObserver {
     Map<QueryEngine, Collection<Query>> queriesByNode;
     Map<Query, Query> queriesByExecutedQueries;
     boolean noObjectConversion;
+    boolean cachedResult;
 
     /*
      * A constructor for the "new" way of performing a query via 'execute' with
@@ -412,6 +414,7 @@ class DataDomainQueryAction implements QueryRouter, 
OperationObserver {
             // cache
             queryCache.put(metadata, factory.createObject());
         }
+        cachedResult = true;
 
         return DONE;
     }
@@ -772,38 +775,71 @@ class DataDomainQueryAction implements QueryRouter, 
OperationObserver {
 
         @Override
         void convert(List<Object[]> mainRows) {
+            if (mainRows.isEmpty()) {
+                // just a sanity check, should not be a valid case
+                return;
+            }
 
-            int rowsLen = mainRows.size();
+            // do we have anything to convert to objects inside mainRows?
+            boolean needConversion = needConversion();
 
-            List<Object> rsMapping = metadata.getResultSetMapping();
-            int width = rsMapping.size();
+            // create result list
+            List<Object[]> result = createResultList(mainRows, needConversion);
 
             // no conversions needed for scalar positions;
-            // reuse Object[]'s to fill them with resolved objects
+            if(needConversion) {
+                // reuse Object[]'s to fill them with resolved objects
+                List<PrefetchProcessorNode> segmentNodes = 
doInPlaceConversion(result);
+
+                // invoke callbacks now that all objects are resolved...
+                LifecycleCallbackRegistry callbackRegistry = 
context.getEntityResolver().getCallbackRegistry();
+                if (!callbackRegistry.isEmpty(LifecycleEvent.POST_LOAD)) {
+                    for (PrefetchProcessorNode node : segmentNodes) {
+                        performPostLoadCallbacks(node, callbackRegistry);
+                    }
+                }
+            }
+
+            // distinct filtering
+            if (!metadata.isSuppressingDistinct()) {
+                Set<List<?>> seen = new HashSet<>(result.size());
+                result.removeIf(objects -> !seen.add(Arrays.asList(objects)));
+            }
+
+            updateResponse(mainRows, result);
+        }
+
+        private List<PrefetchProcessorNode> doInPlaceConversion(List<Object[]> 
result) {
+            List<Object> resultSetMapping = metadata.getResultSetMapping();
+            int width = resultSetMapping.size();
+            int height  = result.size();
             List<PrefetchProcessorNode> segmentNodes = new ArrayList<>(width);
             for (int i = 0; i < width; i++) {
-                Object mapping = rsMapping.get(i);
+                Object mapping = resultSetMapping.get(i);
                 if (mapping instanceof EntityResultSegment) {
                     EntityResultSegment entitySegment = (EntityResultSegment) 
mapping;
                     PrefetchProcessorNode nextResult = 
toResultsTree(entitySegment.getClassDescriptor(),
-                            metadata.getPrefetchTree(), mainRows, i);
+                            metadata.getPrefetchTree(), result, i);
 
                     segmentNodes.add(nextResult);
 
                     List<Persistent> objects = nextResult.getObjects();
-
-                    for (int j = 0; j < rowsLen; j++) {
-                        Object[] row = mainRows.get(j);
+                    for (int j = 0; j < height; j++) {
+                        Object[] row = result.get(j);
                         row[i] = objects.get(j);
                     }
                 } else if (mapping instanceof EmbeddableResultSegment) {
-                    EmbeddableResultSegment resultSegment = 
(EmbeddableResultSegment)mapping;
+                    EmbeddableResultSegment resultSegment = 
(EmbeddableResultSegment) mapping;
                     Embeddable embeddable = resultSegment.getEmbeddable();
-                    Class<?> embeddableClass = 
objectFactory.getJavaClass(embeddable.getClassName());
+                    @SuppressWarnings("unchecked")
+                    Class<? extends EmbeddableObject> embeddableClass = 
(Class<? extends EmbeddableObject>) objectFactory
+                            .getJavaClass(embeddable.getClassName());
                     try {
-                        for(Object[] row : mainRows) {
-                            DataRow dataRow = (DataRow)row[i];
-                            EmbeddableObject eo = 
(EmbeddableObject)embeddableClass.newInstance();
+                        Constructor<? extends EmbeddableObject> 
declaredConstructor = embeddableClass
+                                .getDeclaredConstructor();
+                        for (Object[] row : result) {
+                            DataRow dataRow = (DataRow) row[i];
+                            EmbeddableObject eo = 
declaredConstructor.newInstance();
                             dataRow.forEach(eo::writePropertyDirectly);
                             row[i] = eo;
                         }
@@ -812,20 +848,36 @@ class DataDomainQueryAction implements QueryRouter, 
OperationObserver {
                     }
                 }
             }
+            return segmentNodes;
+        }
 
-            if(!metadata.isSuppressingDistinct()) {
-                Set<List<?>> seen = new HashSet<>(mainRows.size());
-                mainRows.removeIf(objects -> 
!seen.add(Arrays.asList(objects)));
+        private List<Object[]> createResultList(List<Object[]> mainRows, 
boolean needConversion) {
+            if(!cachedResult) {
+                // fast-path, we can reuse existing rows
+                return mainRows;
             }
 
-            // invoke callbacks now that all objects are resolved...
-            LifecycleCallbackRegistry callbackRegistry = 
context.getEntityResolver().getCallbackRegistry();
+            if(!needConversion) {
+                // no conversion needed, so can clone only top-level list
+                return new ArrayList<>(mainRows);
+            }
 
-            if (!callbackRegistry.isEmpty(LifecycleEvent.POST_LOAD)) {
-                for (PrefetchProcessorNode node : segmentNodes) {
-                    performPostLoadCallbacks(node, callbackRegistry);
+            // slowest path, deep copy everything
+            List<Object[]> result = new ArrayList<>(mainRows.size());
+            for(Object[] row : mainRows) {
+                result.add(Arrays.copyOf(row, 
metadata.getResultSetMapping().size()));
+            }
+            return result;
+        }
+
+        private boolean needConversion() {
+            for (Object mapping : metadata.getResultSetMapping()) {
+                if (mapping instanceof EntityResultSegment
+                        || mapping instanceof EmbeddableResultSegment) {
+                    return true;
                 }
             }
+            return false;
         }
     }
 
diff --git 
a/cayenne-server/src/test/java/org/apache/cayenne/query/ColumnSelectIT.java 
b/cayenne-server/src/test/java/org/apache/cayenne/query/ColumnSelectIT.java
index f9496e020..21430594c 100644
--- a/cayenne-server/src/test/java/org/apache/cayenne/query/ColumnSelectIT.java
+++ b/cayenne-server/src/test/java/org/apache/cayenne/query/ColumnSelectIT.java
@@ -59,6 +59,7 @@ import org.junit.Ignore;
 import org.junit.Test;
 
 import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.*;
 
 /**
@@ -1195,6 +1196,24 @@ public class ColumnSelectIT extends ServerCase {
 
     }
 
+    @Test
+    public void testSharedCache() {
+        ColumnSelect<Object[]> query = ObjectSelect.query(Artist.class)
+                .columns(Artist.ARTIST_NAME, Artist.DATE_OF_BIRTH, 
PropertyFactory.createSelf(Artist.class))
+                .orderBy(Artist.ARTIST_ID_PK_PROPERTY.asc())
+                .cacheStrategy(QueryCacheStrategy.SHARED_CACHE);
+
+        List<Object[]> result = query.select(context);
+        assertEquals(20, result.size());
+        assertThat("Should be an instance of Artist",
+                instanceOf(Artist.class).matches(result.get(0)[2]));
+
+        List<Object[]> result2 = query.select(context);
+        assertEquals(20, result2.size());
+        assertThat("Should be an instance of Artist",
+                instanceOf(Artist.class).matches(result.get(0)[2]));
+    }
+
     static class TestPojo {
         String name;
         Date date;

Reply via email to