https://github.com/manifold-systems/manifold/issues/631

- several improvements to schema API naming -- improve pascal-case naming e.g., retain existing mixed case names instead of flattening them -- when a table and a view map to the same API name, let the table keep the preferred API name and set the view name to "<api-name>_view" -- otherwise if tables/views map to the same API name, generate names in sequence "<api-name>_1", "<api-name>_2", etc.
manifold-systems · Oct 22, 2024 · b5fd7b0 · b5fd7b0
1 parent 1eac5e6
commit b5fd7b0
Show file tree

Hide file tree

Showing 6 changed files with 142 additions and 21 deletions.
diff --git a/manifold-core-parent/manifold-rt/src/main/java/manifold/rt/api/util/ManIdentifierUtil.java b/manifold-core-parent/manifold-rt/src/main/java/manifold/rt/api/util/ManIdentifierUtil.java
@@ -57,10 +57,50 @@ else if( c == '_' ||
     return identifier;
   }
 
+  /**
+   * Return {@code name} following Pascal naming convention. Only alpha-numeric characters are retained.
+   * <p/>
+   * - name -> Name <br>
+   * - NAME -> Name <br>
+   * - thisName -> ThisName <br>
+   * - thisname -> Thisname <br>
+   * - this_name -> ThisName <br>
+   * - this-name -> ThisName <br>
+   * - this*name -> ThisName <br>
+   * - _this_name -> _ThisName <br>
+   * - _this__name -> _This_Name <br>
+   *
+   * @param name Any name
+   * @param firstChar Should the first character be capitalized? (false = camel case)
+   * @return {@code name} following pascal naming convention
+   */
   public static String makePascalCaseIdentifier( String name, boolean firstChar )
   {
-    // pascal case remove underscores, great for transforming DDL names
-    return ManStringUtil.toPascalCase( ManIdentifierUtil.makeIdentifier( name ).toLowerCase(), !firstChar );
+    return ManStringUtil.toPascalCase( isMixedCase( name ) ? makeIdentifier( name ) : makeIdentifier( name ).toLowerCase(), !firstChar );
+  }
+
+  private static boolean isMixedCase( String name )
+  {
+    if( name.contains( "_" ) )
+    {
+      return false;
+    }
+
+    Boolean lower = null;
+    for( int i = 0; i < name.length(); i++ )
+    {
+      char c = name.charAt( i );
+      if( Character.isAlphabetic( c ) )
+      {
+        boolean l = Character.isLowerCase( c );
+        if( lower != null && l != lower )
+        {
+          return true;
+        }
+        lower = l;
+      }
+    }
+    return false;
   }
 
   private static String makeCorrections( StringBuilder sb )

diff --git a/...old-core-parent/manifold/src/main/java/manifold/internal/javac/CompiledTypeProcessor.java b/...old-core-parent/manifold/src/main/java/manifold/internal/javac/CompiledTypeProcessor.java
@@ -305,7 +305,7 @@ private void preserveInnerClassesForGeneration( JCTree.JCClassDecl tree )
 
   public void preserveInnerClassForGenerationPhase( JCTree.JCClassDecl def )
   {
-    if( def == null )
+    if( def == null || def.sym == null )
     {
       return;
     }

diff --git a/...manifold-sql-inproc-test/src/test/java/manifold/sql/schema/duckdb/types/AllTypesTest.java b/...manifold-sql-inproc-test/src/test/java/manifold/sql/schema/duckdb/types/AllTypesTest.java
@@ -70,17 +70,17 @@ public class AllTypesTest extends DuckdbDdlServerTest
   public void testTypesThatAreWriteBroken() throws SQLException
   {
     // ARRAY
-    Object[] array = (Object[]) "[.sql:DuckdbSakila/] SELECT array_value(1, 2, 3) as arraySample".fetchOne().getArraysample();
+    Object[] array = (Object[]) "[.sql:DuckdbSakila/] SELECT array_value(1, 2, 3) as arraySample".fetchOne().getArraySample();
     for( Object o : array ) {
       out.println(o.getClass());
     }
 
     // STRUCT
-    DuckDBStruct structSample = "[.sql:DuckdbSakila/] SELECT {'x': 1, 'y': 2, 'z': 3} as structSample".fetchOne().getStructsample();
+    DuckDBStruct structSample = "[.sql:DuckdbSakila/] SELECT {'x': 1, 'y': 2, 'z': 3} as structSample".fetchOne().getStructSample();
     structSample.getMap().forEach((key, value) -> out.println(key + ": " + value));
 
     // MAP
-    Map<?,?> mapSample = "[.sql:DuckdbSakila/] SELECT MAP {'x': 1, 'y': 2, 'z': 3} as mapSample".fetchOne().getMapsample();
+    Map<?,?> mapSample = "[.sql:DuckdbSakila/] SELECT MAP {'x': 1, 'y': 2, 'z': 3} as mapSample".fetchOne().getMapSample();
     mapSample.forEach((key, value) -> out.println(key + ": " + value));
 
     // BIT (BITSTRING)

diff --git a/...test/java/manifold/sql/util/UtilTest.java → ...ifold/sql/util/ManIdentifierUtilTest.java b/...test/java/manifold/sql/util/UtilTest.java → ...ifold/sql/util/ManIdentifierUtilTest.java
@@ -16,13 +16,12 @@
 
 package manifold.sql.util;
 
-import manifold.rt.api.util.ManIdentifierUtil;
 import org.junit.Test;
 
 import static manifold.rt.api.util.ManIdentifierUtil.makePascalCaseIdentifier;
 import static org.junit.Assert.*;
 
-public class UtilTest
+public class ManIdentifierUtilTest
 {
   @Test
   public void testPascalCase()
@@ -47,5 +46,13 @@ public void testPascalCase()
     assertEquals( "cityId", ident );
     ident = makePascalCaseIdentifier( "city#id", true );
     assertEquals( "CityId", ident );
+    ident = makePascalCaseIdentifier( "cityid", false );
+    assertEquals( "cityid", ident );
+    ident = makePascalCaseIdentifier( "cityid", true );
+    assertEquals( "Cityid", ident );
+    ident = makePascalCaseIdentifier( "retainMixedCase", false );
+    assertEquals( "retainMixedCase", ident );
+    ident = makePascalCaseIdentifier( "retainMixedCase", true );
+    assertEquals( "RetainMixedCase", ident );
   }
 }
diff --git a/manifold-deps-parent/manifold-sql-rt/src/main/java/manifold/sql/rt/api/ValueAccessor.java b/manifold-deps-parent/manifold-sql-rt/src/main/java/manifold/sql/rt/api/ValueAccessor.java
@@ -16,6 +16,7 @@
 
 package manifold.sql.rt.api;
 
+import manifold.util.ReflectUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -82,17 +83,15 @@ default String getParameterExpression( DatabaseMetaData metaData, Object value,
    */
   default Class<?> getClassForColumnClassName( String className, Class<?> defaultClass )
   {
+    Class<?> result = null;
     if( className != null && !className.equals( Object.class.getTypeName() ) )
     {
-      try
+      result = ReflectUtil.type( className );
+      if( result == null)
       {
-        return Class.forName( className );
-      }
-      catch( ClassNotFoundException cnfe )
-      {
-        LOGGER.warn( "Failed to access class '" + className + "' for '" + getClass().getSimpleName() + "'", cnfe );
+        LOGGER.warn( "Failed to access class '" + className + "' for '" + getClass().getSimpleName() + "'" );
       }
     }
-    return defaultClass;
+    return result == null ? defaultClass : result;
   }
 }
diff --git a/manifold-deps-parent/manifold-sql/src/main/java/manifold/sql/schema/jdbc/JdbcSchema.java b/manifold-deps-parent/manifold-sql/src/main/java/manifold/sql/schema/jdbc/JdbcSchema.java
@@ -108,17 +108,14 @@ private void build( Connection c, DatabaseMetaData metaData ) throws SQLExceptio
     String catalog = _schemaIsCatalog ? _name : _dbConfig.getCatalogName();
     String schema = _schemaIsCatalog ? null : _name;
     LOGGER.info( "JdbcSchema building: catalog: " + catalog + ", schema: " + schema );
-    assignTableNames( catalog, schema, metaData );
+    assignTableNames( catalog, schema, metaData ); // necessary to assign table names before creating tables due to forward refs
     try( ResultSet resultSet = metaData.getTables( catalog, schema, getTableNamePattern(), getTableTableTypes( metaData ) ) )
     {
       while( resultSet.next() )
       {
         JdbcSchemaTable table = new JdbcSchemaTable( this, metaData, resultSet );
         String name = table.getName();
         _tables.put( name, table );
-        String javaName = makePascalCaseIdentifier( name, true );
-        _javaToName.put( javaName, name );
-        _nameToJava.put( name, javaName );
       }
     }
     catch( SQLException e )
@@ -140,21 +137,99 @@ private void build( Connection c, DatabaseMetaData metaData ) throws SQLExceptio
 
   private void assignTableNames( String catalog, String schema, DatabaseMetaData metaData )
   {
+    Map<String, List<TableData>> tables = new LinkedHashMap<>();
     try( ResultSet resultSet = metaData.getTables( catalog, schema, getTableNamePattern(), getTableTableTypes( metaData ) ) )
     {
       while( resultSet.next() )
       {
         String name = resultSet.getString( "TABLE_NAME" );
+        String type = resultSet.getString( "TABLE_TYPE" );
         String javaName = makePascalCaseIdentifier( name, true );
-        _javaToName.put( javaName, name );
-        _nameToJava.put( name, javaName );
+        tables.computeIfAbsent( javaName, __ -> new ArrayList<>() )
+          .add( new TableData( name, type ) );
       }
     }
     catch( SQLException e )
     {
       // this is dicey, but some drivers (looking at you duckdb) appear to fail after successfully iterating this resultset
       LOGGER.warn( "JdbcSchema: build may not have completed.", e );
     }
+
+    for( Map.Entry<String, List<TableData>> entry: tables.entrySet() )
+    {
+      String javaName = entry.getKey();
+      List<TableData> list = entry.getValue();
+      if( list.size() == 1 )
+      {
+        // only one table mapped to the java name, assign the name as usual
+
+        TableData tableData = list.get( 0 );
+        _javaToName.put( javaName, tableData.rawName );
+        _nameToJava.put( tableData.rawName, javaName );
+      }
+      else if( list.size() == 2 && !list.get( 0 ).type.equals( list.get( 1 ).type ) )
+      {
+        // for case where a table and a view resolve to the same java name,
+        // reserve the java name for the table and make a new one for the view
+
+        TableData first = list.get( 0 );
+        TableData second = list.get( 1 );
+        TableData table = null;
+        TableData view = null;
+        if( first.type.toLowerCase().contains( "view" ) )
+        {
+          table = second;
+          view = first;
+        }
+        else if( second.type.toLowerCase().contains( "view" ) )
+        {
+          table = first;
+          view = second;
+        }
+
+        if( view != null )
+        {
+          _javaToName.put( javaName, table.rawName );
+          _nameToJava.put( table.rawName, javaName );
+
+          javaName += "_view";
+          _javaToName.put( javaName, view.rawName );
+          _nameToJava.put( view.rawName, javaName );
+        }
+        else
+        {
+          simpleNameChange( list, javaName );
+        }
+      }
+      else
+      {
+        // more than two tables/views resolve to the same java name, just append _1 etc. to the java names
+        simpleNameChange( list, javaName );
+      }
+    }
+  }
+
+  private void simpleNameChange( List<TableData> list, String javaName )
+  {
+    for( int i = 0; i < list.size(); i++ )
+    {
+      TableData tableData = list.get( i );
+      javaName += "_" + (i+1);
+      _javaToName.put( javaName, tableData.rawName );
+      _nameToJava.put( tableData.rawName, javaName );
+    }
+  }
+
+  private static class TableData
+  {
+    private final String rawName;
+    private final String type;
+
+    TableData( String name, String type )
+    {
+      this.rawName = name;
+      this.type = type;
+    }
   }
 
   private String getTableNamePattern()