cayenne-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ntimof...@apache.org
Subject cayenne git commit: CAY-2208 SQLTemplate: LEFT JOIN to a subset of a table returns nulls for entries that don't have a match in the subset - validate duplicated columns and warn - minor code cleanup
Date Tue, 11 Apr 2017 10:02:07 GMT
Repository: cayenne
Updated Branches:
  refs/heads/master 0bf39d05e -> c9c7cbed6


CAY-2208 SQLTemplate: LEFT JOIN to a subset of a table returns nulls for entries that don't
have a match in the subset
  - validate duplicated columns and warn
  - minor code cleanup


Project: http://git-wip-us.apache.org/repos/asf/cayenne/repo
Commit: http://git-wip-us.apache.org/repos/asf/cayenne/commit/c9c7cbed
Tree: http://git-wip-us.apache.org/repos/asf/cayenne/tree/c9c7cbed
Diff: http://git-wip-us.apache.org/repos/asf/cayenne/diff/c9c7cbed

Branch: refs/heads/master
Commit: c9c7cbed641fe1600181511481e191a98da61ae6
Parents: 0bf39d0
Author: stariy <stariy95@gmail.com>
Authored: Tue Apr 11 13:02:01 2017 +0300
Committer: stariy <stariy95@gmail.com>
Committed: Tue Apr 11 13:02:01 2017 +0300

----------------------------------------------------------------------
 .../apache/cayenne/access/ObjectResolver.java   |  3 +-
 .../access/jdbc/RowDescriptorBuilder.java       | 42 +++++++++++++++++++-
 .../cayenne/access/jdbc/SQLTemplateAction.java  | 31 +++++----------
 .../jdbc/reader/DefaultRowReaderFactory.java    |  2 +-
 .../access/jdbc/reader/EntityRowReader.java     |  7 ++--
 .../access/jdbc/SQLTemplateActionIT.java        | 25 ++++++++++++
 docs/doc/src/main/resources/RELEASE-NOTES.txt   |  1 +
 7 files changed, 82 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cayenne/blob/c9c7cbed/cayenne-server/src/main/java/org/apache/cayenne/access/ObjectResolver.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/ObjectResolver.java b/cayenne-server/src/main/java/org/apache/cayenne/access/ObjectResolver.java
index 56b52b5..eb0c801 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/ObjectResolver.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/ObjectResolver.java
@@ -127,8 +127,7 @@ class ObjectResolver {
 		ClassDescriptor classDescriptor = descriptorResolutionStrategy.descriptorForRow(row);
 
 		// not using DataRow.createObjectId for performance reasons -
-		// ObjectResolver
-		// has all needed metadata already cached.
+		// ObjectResolver has all needed metadata already cached.
 		ObjectId anId = createObjectId(row, classDescriptor.getEntity(), null);
 		return objectFromDataRow(row, anId, classDescriptor);
 	}

http://git-wip-us.apache.org/repos/asf/cayenne/blob/c9c7cbed/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/RowDescriptorBuilder.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/RowDescriptorBuilder.java
b/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/RowDescriptorBuilder.java
index 8cf71e8..235e804 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/RowDescriptorBuilder.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/RowDescriptorBuilder.java
@@ -21,13 +21,20 @@ package org.apache.cayenne.access.jdbc;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.cayenne.CayenneRuntimeException;
 import org.apache.cayenne.access.types.ExtendedType;
 import org.apache.cayenne.access.types.ExtendedTypeMap;
 import org.apache.commons.collections.Transformer;
+import org.apache.commons.lang.StringUtils;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 /**
  * A builder class that helps to assemble {@link RowDescriptor} instances from various
@@ -37,6 +44,8 @@ import org.apache.commons.collections.Transformer;
  */
 public class RowDescriptorBuilder {
 
+    private static final Log logger = LogFactory.getLog(RowDescriptorBuilder.class);
+
     private static final Transformer UPPERCASE_TRANSFORMER = new Transformer() {
 
         public Object transform(Object input) {
@@ -57,6 +66,8 @@ public class RowDescriptorBuilder {
     protected Transformer caseTransformer;
     protected Map<String, String> typeOverrides;
 
+    protected boolean validateDuplicateColumnNames;
+
     /**
      * Returns a RowDescriptor built based on the builder internal state.
      */
@@ -107,15 +118,35 @@ public class RowDescriptorBuilder {
         }
 
         ColumnDescriptor[] rsColumns = new ColumnDescriptor[rsLen];
+        List<String> duplicates = null;
+        Set<String> uniqueNames = null;
+        if(validateDuplicateColumnNames) {
+            duplicates = new ArrayList<>();
+            uniqueNames = new HashSet<>();
+        }
 
         int outputLen = 0;
         for (int i = 0; i < rsLen; i++) {
             String rowkey = resolveDataRowKeyFromResultSet(i + 1);
             
             // resolve column descriptor from 'columns' or create new
-            rsColumns[outputLen] = getColumnDescriptor(rowkey, columns, i + 1);
+            ColumnDescriptor descriptor = getColumnDescriptor(rowkey, columns, i + 1);
+
+            // validate uniqueness of names
+            if(validateDuplicateColumnNames) {
+                if(!uniqueNames.add(descriptor.getDataRowKey())) {
+                    duplicates.add(descriptor.getDataRowKey());
+                }
+            }
+            rsColumns[outputLen] = descriptor;
             outputLen++;
         }
+
+        if(validateDuplicateColumnNames && !duplicates.isEmpty()) {
+            logger.warn("Found duplicated columns '" + StringUtils.join(duplicates, "', '")
+ "' in row descriptor. " +
+                    "This can lead to errors when converting result to persistent objects.");
+        }
+
         if (outputLen < rsLen) {
             // cut ColumnDescriptor array
             ColumnDescriptor[] rsColumnsCut = new ColumnDescriptor[outputLen];
@@ -222,6 +253,15 @@ public class RowDescriptorBuilder {
         return this;
     }
 
+    /**
+     * Validate and report duplicate names of columns.
+     * @return this builder
+     */
+    public RowDescriptorBuilder validateDuplicateColumnNames() {
+        this.validateDuplicateColumnNames = true;
+        return this;
+    }
+
     public boolean isOverriden(String columnName) {
         return typeOverrides != null && typeOverrides.containsKey(columnName);
     }

http://git-wip-us.apache.org/repos/asf/cayenne/blob/c9c7cbed/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java
b/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java
index e8c1cf7..a7d502c 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java
@@ -116,8 +116,7 @@ public class SQLTemplateAction implements SQLAction {
 		}
 
 		// notify of combined counts of all queries inside SQLTemplate
-		// multiplied by the
-		// number of parameter sets...
+		// multiplied by the number of parameter sets...
 		int[] ints = new int[counts.size()];
 		for (int i = 0; i < ints.length; i++) {
 			ints[i] = counts.get(i).intValue();
@@ -201,9 +200,7 @@ public class SQLTemplateAction implements SQLAction {
 							}
 						}
 
-						// ignore possible following update counts and bail
-						// early on
-						// iterated results
+						// ignore possible following update counts and bail early on iterated results
 						if (iteratedResult) {
 							break;
 						}
@@ -274,20 +271,18 @@ public class SQLTemplateAction implements SQLAction {
 	 */
 	protected RowDescriptorBuilder configureRowDescriptorBuilder(SQLStatement compiled, ResultSet
resultSet)
 			throws SQLException {
-		RowDescriptorBuilder builder = new RowDescriptorBuilder();
-		builder.setResultSet(resultSet);
+		RowDescriptorBuilder builder = new RowDescriptorBuilder()
+                .setResultSet(resultSet)
+                .validateDuplicateColumnNames();
 
-		// SQLTemplate #result columns take precedence over other ways to
-		// determine the
-		// type
+		// SQLTemplate #result columns take precedence over other ways to determine the type
 		if (compiled.getResultColumns().length > 0) {
 			builder.setColumns(compiled.getResultColumns());
 		}
 
 		ObjEntity entity = queryMetadata.getObjEntity();
 		if (entity != null) {
-			// TODO: andrus 2008/03/28 support flattened attributes with
-			// aliases...
+			// TODO: andrus 2008/03/28 support flattened attributes with aliases...
 			for (ObjAttribute attribute : entity.getAttributes()) {
 				String column = attribute.getDbAttributePath();
 				if (column == null || column.indexOf('.') > 0) {
@@ -297,16 +292,12 @@ public class SQLTemplateAction implements SQLAction {
 			}
 		}
 
-		// override numeric Java types based on JDBC defaults for DbAttributes,
-		// as
-		// Oracle
+		// override numeric Java types based on JDBC defaults for DbAttributes, as Oracle
 		// ResultSetMetadata is not very precise about NUMERIC distinctions...
 		// (BigDecimal vs Long vs. Integer)
 		if (dbEntity != null) {
 			for (DbAttribute attribute : dbEntity.getAttributes()) {
-
 				if (!builder.isOverriden(attribute.getName()) && TypesMapping.isNumeric(attribute.getType()))
{
-
 					builder.overrideColumnType(attribute.getName(), TypesMapping.getJavaBySqlType(attribute.getType()));
 				}
 			}
@@ -333,10 +324,8 @@ public class SQLTemplateAction implements SQLAction {
 	protected String extractTemplateString() {
 		String sql = query.getTemplate(dbAdapter.getClass().getName());
 
-		// note that we MUST convert line breaks to spaces. On some databases
-		// (DB2)
-		// queries with breaks simply won't run; the rest are affected by
-		// CAY-726.
+		// note that we MUST convert line breaks to spaces. On some databases (DB2)
+		// queries with breaks simply won't run; the rest are affected by CAY-726.
 		return Util.stripLineBreaks(sql, ' ');
 	}
 

http://git-wip-us.apache.org/repos/asf/cayenne/blob/c9c7cbed/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/reader/DefaultRowReaderFactory.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/reader/DefaultRowReaderFactory.java
b/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/reader/DefaultRowReaderFactory.java
index 73a29e7..02ba45a 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/reader/DefaultRowReaderFactory.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/reader/DefaultRowReaderFactory.java
@@ -84,7 +84,7 @@ public class DefaultRowReaderFactory implements RowReaderFactory {
 							createEntityRowReader(descriptor, queryMetadata, (EntityResultSegment) segment,
 									postProcessorFactory));
 				} else {
-					reader.addRowReader(i, new ScalarRowReader<Object>(descriptor, (ScalarResultSegment)
segment));
+					reader.addRowReader(i, new ScalarRowReader<>(descriptor, (ScalarResultSegment)
segment));
 				}
 			}
 

http://git-wip-us.apache.org/repos/asf/cayenne/blob/c9c7cbed/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/reader/EntityRowReader.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/reader/EntityRowReader.java
b/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/reader/EntityRowReader.java
index ccabb81..fff1bf9 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/reader/EntityRowReader.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/jdbc/reader/EntityRowReader.java
@@ -66,19 +66,18 @@ class EntityRowReader implements RowReader<DataRow> {
             this.converters[i] = converters[startIndex + i];
             types[i] = columns[startIndex + i].getJdbcType();
 
-            // query translator may change the order of fields compare to the
-            // entity
+            // query translator may change the order of fields compare to the entity
             // result, so figure out DataRow labels by doing reverse lookup of
             // RowDescriptor labels...
             if (columns[startIndex + i].getDataRowKey().contains(".")) {
                 // if the dataRowKey contains ".", it is prefetched column and
-                // we can use
-                // it instead of search the name by alias
+                // we can use it instead of search the name by alias
                 labels[i] = columns[startIndex + i].getDataRowKey();
             } else {
                 labels[i] = segmentMetadata.getColumnPath(columns[startIndex + i].getDataRowKey());
             }
         }
+        this.mapCapacity = (int) Math.ceil(segmentWidth / 0.75);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cayenne/blob/c9c7cbed/cayenne-server/src/test/java/org/apache/cayenne/access/jdbc/SQLTemplateActionIT.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/test/java/org/apache/cayenne/access/jdbc/SQLTemplateActionIT.java
b/cayenne-server/src/test/java/org/apache/cayenne/access/jdbc/SQLTemplateActionIT.java
index b025b86..22d0eda 100644
--- a/cayenne-server/src/test/java/org/apache/cayenne/access/jdbc/SQLTemplateActionIT.java
+++ b/cayenne-server/src/test/java/org/apache/cayenne/access/jdbc/SQLTemplateActionIT.java
@@ -37,6 +37,7 @@ import org.apache.cayenne.access.DataNode;
 import org.apache.cayenne.access.MockOperationObserver;
 import org.apache.cayenne.dba.JdbcAdapter;
 import org.apache.cayenne.di.Inject;
+import org.apache.cayenne.query.CapsStrategy;
 import org.apache.cayenne.query.Query;
 import org.apache.cayenne.query.SQLAction;
 import org.apache.cayenne.query.SQLTemplate;
@@ -45,6 +46,7 @@ import org.apache.cayenne.query.SortOrder;
 import org.apache.cayenne.test.jdbc.DBHelper;
 import org.apache.cayenne.test.jdbc.TableHelper;
 import org.apache.cayenne.testdo.testmap.Artist;
+import org.apache.cayenne.unit.UnitDbAdapter;
 import org.apache.cayenne.unit.di.server.CayenneProjects;
 import org.apache.cayenne.unit.di.server.ServerCase;
 import org.apache.cayenne.unit.di.server.ServerCaseDataSourceFactory;
@@ -67,6 +69,9 @@ public class SQLTemplateActionIT extends ServerCase {
 	protected JdbcAdapter adapter;
 
 	@Inject
+	protected UnitDbAdapter unitDbAdapter;
+
+	@Inject
 	protected ObjectContext objectContext;
 
 	@Inject
@@ -142,6 +147,26 @@ public class SQLTemplateActionIT extends ServerCase {
 	}
 
 	@Test
+	public void selectObjects() throws Exception {
+		createFourArtists();
+
+		String templateString = "SELECT * FROM ARTIST";
+		SQLTemplate sqlTemplate = new SQLTemplate(Artist.class, templateString);
+
+		if(unitDbAdapter.isLowerCaseNames()) {
+			sqlTemplate.setColumnNamesCapitalization(CapsStrategy.UPPER);
+		}
+
+		@SuppressWarnings("unchecked")
+		List<Artist> artists = (List<Artist>)objectContext.performQuery(sqlTemplate);
+
+		assertEquals(4, artists.size());
+		for(Artist artist : artists){
+			assertTrue(artist.getArtistName().startsWith("artist"));
+		}
+	}
+
+	@Test
 	public void testSelectUtilDate() throws Exception {
 		createFourArtists();
 

http://git-wip-us.apache.org/repos/asf/cayenne/blob/c9c7cbed/docs/doc/src/main/resources/RELEASE-NOTES.txt
----------------------------------------------------------------------
diff --git a/docs/doc/src/main/resources/RELEASE-NOTES.txt b/docs/doc/src/main/resources/RELEASE-NOTES.txt
index dbd3358..31b50af 100644
--- a/docs/doc/src/main/resources/RELEASE-NOTES.txt
+++ b/docs/doc/src/main/resources/RELEASE-NOTES.txt
@@ -39,6 +39,7 @@ CAY-2077 Bug in CayenneRuntimeException using wrong specified string in
Formatte
 CAY-2094 SelectById query doesn't work from ROP client
 CAY-2161 'Not for Client Use' option is ignored at Class Generation
 CAY-2171 Modeler: Undo db Entity Sync throws error
+CAY-2208 SQLTemplate: LEFT JOIN to a subset of a table returns nulls for entries that don't
have a match in the subset
 CAY-2230 Error using connection to postgresql with db schema in DB URL
 CAY-2240 Modeler: issue with cursor rendering for EJBQL query
 CAY-2243 ObjectContext.getGraphManager().unregisterObject() inconsistencies


Mime
View raw message