fixed bug 35474, fixed issue 35519: function based indexes improvements
authorAsier Lostalé <asier.lostale@openbravo.com>
Wed, 15 Mar 2017 13:35:00 +0100
changeset 930 2686a1fd45b9
parent 929 2c18bf970368
child 931 839d66eb2879
fixed bug 35474, fixed issue 35519: function based indexes improvements

Improved the way function definitions in indexes are read from DB, now they are
read per column, using in PG the index specific function pg_get_indexdef. This
allows to:

* properly read indexes based on arithmethic expressions ie (col1-col2)
* support more than one index column based on function
src-test/model/indexes/FUNCTION_INDEX_MULTI_COL.xml
src-test/model/indexes/FUNCTION_INDEX_OPERATOR.xml
src-test/src/org/openbravo/dbsm/test/model/FunctionBasedIndexes.java
src/org/apache/ddlutils/platform/SqlBuilder.java
src/org/apache/ddlutils/platform/oracle/OracleModelLoader.java
src/org/apache/ddlutils/platform/postgresql/PostgreSqlModelLoader.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src-test/model/indexes/FUNCTION_INDEX_MULTI_COL.xml	Wed Mar 15 13:35:00 2017 +0100
@@ -0,0 +1,21 @@
+<?xml version="1.0"?>
+  <database name="TABLE TEST">
+    <table name="TEST" primaryKey="TEST_ID">
+      <column name="TEST_ID" primaryKey="true" required="true" type="VARCHAR" size="32" autoIncrement="false">
+        <default/>
+        <onCreateDefault/>
+      </column>
+      <column name="COL1" primaryKey="false" required="true" type="VARCHAR" size="32" autoIncrement="false">
+        <default/>
+        <onCreateDefault/>
+      </column>
+      <column name="COL2" primaryKey="false" required="true" type="VARCHAR" size="32" autoIncrement="false">
+        <default/>
+        <onCreateDefault/>
+      </column>
+      <index name="TEST_MULTICOL" unique="false">
+        <index-column name="functionBasedColumn" functionExpression="UPPER(COL1)"/>
+        <index-column name="functionBasedColumn" functionExpression="LOWER(COL2)"/>
+      </index>
+    </table>
+  </database>
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src-test/model/indexes/FUNCTION_INDEX_OPERATOR.xml	Wed Mar 15 13:35:00 2017 +0100
@@ -0,0 +1,20 @@
+<?xml version="1.0"?>
+  <database name="TABLE TEST">
+    <table name="TEST" primaryKey="TEST_ID">
+      <column name="TEST_ID" primaryKey="true" required="true" type="VARCHAR" size="32" autoIncrement="false">
+        <default/>
+        <onCreateDefault/>
+      </column>
+      <column name="COL1" primaryKey="false" required="true" type="DECIMAL" size="10,0" autoIncrement="false">
+        <default/>
+        <onCreateDefault/>
+      </column>
+      <column name="COL2" primaryKey="false" required="true" type="DECIMAL" size="10,0" autoIncrement="false">
+        <default/>
+        <onCreateDefault/>
+      </column>
+      <index name="OP_INDEX" unique="false">
+        <index-column name="functionBasedColumn" functionExpression="COL1-COL2"/>
+      </index>
+    </table>
+  </database>
--- a/src-test/src/org/openbravo/dbsm/test/model/FunctionBasedIndexes.java	Mon Mar 13 14:08:29 2017 +0100
+++ b/src-test/src/org/openbravo/dbsm/test/model/FunctionBasedIndexes.java	Wed Mar 15 13:35:00 2017 +0100
@@ -206,6 +206,32 @@
     assertExport("indexes/BASIC_INDEX.xml", "tables/TEST.xml");
   }
 
+  @Test
+  public void expressionWithOperators() throws IOException {
+    resetDB();
+    updateDatabase("indexes/FUNCTION_INDEX_OPERATOR.xml", false);
+    assertExport("indexes/FUNCTION_INDEX_OPERATOR.xml", "tables/TEST.xml");
+
+    // 2nd update should perform model check, but it doesn't check correctly index type...
+    updateDatabase("indexes/FUNCTION_INDEX_OPERATOR.xml");
+
+    // ...that's why we compare models now
+    assertExport("indexes/FUNCTION_INDEX_OPERATOR.xml", "tables/TEST.xml");
+  }
+
+  @Test
+  public void multiColumnWithExpressions() throws IOException {
+    resetDB();
+    updateDatabase("indexes/FUNCTION_INDEX_MULTI_COL.xml", false);
+    assertExport("indexes/FUNCTION_INDEX_MULTI_COL.xml", "tables/TEST.xml");
+
+    // 2nd update should perform model check, but it doesn't check correctly index type...
+    updateDatabase("indexes/FUNCTION_INDEX_MULTI_COL.xml");
+
+    // ...that's why we compare models now
+    assertExport("indexes/FUNCTION_INDEX_MULTI_COL.xml", "tables/TEST.xml");
+  }
+
   // Given the name of an index, return a string representation of its column, along with the
   // function applied to them
   private String getColumnsFromIndex(String indexName) {
--- a/src/org/apache/ddlutils/platform/SqlBuilder.java	Mon Mar 13 14:08:29 2017 +0100
+++ b/src/org/apache/ddlutils/platform/SqlBuilder.java	Wed Mar 15 13:35:00 2017 +0100
@@ -3606,8 +3606,9 @@
           }
           IndexColumn idxColumn = index.getColumn(idx);
           if ("functionBasedColumn".equals(idxColumn.getName())) {
-            // print the expression instead of just the column name
-            print(idxColumn.getFunctionExpression());
+            // print the expression instead of just the column name, surround it with extra
+            // parenthesis to support ORA-PG compatibility
+            print("(" + idxColumn.getFunctionExpression() + ")");
           } else {
             Column col = table.findColumn(idxColumn.getName());
 
--- a/src/org/apache/ddlutils/platform/oracle/OracleModelLoader.java	Mon Mar 13 14:08:29 2017 +0100
+++ b/src/org/apache/ddlutils/platform/oracle/OracleModelLoader.java	Wed Mar 15 13:35:00 2017 +0100
@@ -1,6 +1,6 @@
 /*
  ************************************************************************************
- * Copyright (C) 2001-2016 Openbravo S.L.U.
+ * Copyright (C) 2001-2017 Openbravo S.L.U.
  * Licensed under the Apache Software License version 2.0
  * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
  * Unless required by applicable law or agreed to  in writing,  software  distributed
@@ -105,16 +105,17 @@
         .prepareStatement("SELECT C.COLUMN_NAME, C2.COLUMN_NAME FROM USER_CONS_COLUMNS C, USER_CONS_COLUMNS C2 WHERE C.CONSTRAINT_NAME = ? and C2.CONSTRAINT_NAME = ? and c.position = c2.position ORDER BY C.POSITION");
 
     _stmt_listindexes = _connection
-        .prepareStatement("SELECT U.INDEX_NAME, U.UNIQUENESS, UE.COLUMN_EXPRESSION, U.TABLE_OWNER FROM USER_INDEXES U LEFT JOIN USER_IND_EXPRESSIONS UE ON U.INDEX_NAME = UE.INDEX_NAME WHERE U.TABLE_NAME = ? AND (U.INDEX_TYPE = 'NORMAL' OR U.INDEX_TYPE = 'FUNCTION-BASED NORMAL') AND NOT EXISTS (SELECT 1 FROM USER_CONSTRAINTS WHERE TABLE_NAME = U.TABLE_NAME AND INDEX_NAME = U.INDEX_NAME AND CONSTRAINT_TYPE IN ('U', 'P')) ORDER BY INDEX_NAME");
+        .prepareStatement("SELECT U.INDEX_NAME, U.UNIQUENESS, U.TABLE_OWNER FROM USER_INDEXES U WHERE U.TABLE_NAME = ? AND (U.INDEX_TYPE = 'NORMAL' OR U.INDEX_TYPE = 'FUNCTION-BASED NORMAL') AND NOT EXISTS (SELECT 1 FROM USER_CONSTRAINTS WHERE TABLE_NAME = U.TABLE_NAME AND INDEX_NAME = U.INDEX_NAME AND CONSTRAINT_TYPE IN ('U', 'P')) ORDER BY INDEX_NAME");
     _stmt_listindexes_prefix = _connection
-        .prepareStatement("SELECT U.INDEX_NAME, U.UNIQUENESS, UE.COLUMN_EXPRESSION, U.TABLE_OWNER FROM USER_INDEXES U LEFT JOIN USER_IND_EXPRESSIONS UE ON U.INDEX_NAME = UE.INDEX_NAME WHERE U.TABLE_NAME = ? AND (U.INDEX_TYPE = 'NORMAL' OR U.INDEX_TYPE = 'FUNCTION-BASED NORMAL') AND NOT EXISTS (SELECT 1 FROM USER_CONSTRAINTS WHERE TABLE_NAME = U.TABLE_NAME AND INDEX_NAME = U.INDEX_NAME AND CONSTRAINT_TYPE IN ('U', 'P')) AND (upper(U.INDEX_NAME) LIKE 'EM_"
+        .prepareStatement("SELECT U.INDEX_NAME, U.UNIQUENESS, U.TABLE_OWNER FROM USER_INDEXES U WHERE U.TABLE_NAME = ? AND (U.INDEX_TYPE = 'NORMAL' OR U.INDEX_TYPE = 'FUNCTION-BASED NORMAL') AND NOT EXISTS (SELECT 1 FROM USER_CONSTRAINTS WHERE TABLE_NAME = U.TABLE_NAME AND INDEX_NAME = U.INDEX_NAME AND CONSTRAINT_TYPE IN ('U', 'P')) AND (upper(U.INDEX_NAME) LIKE 'EM_"
             + _prefix
             + "\\_%' ESCAPE '\\' OR (upper(U.INDEX_NAME)||UPPER(U.TABLE_NAME) IN (SELECT upper(NAME1)||UPPER(NAME2) FROM AD_EXCEPTIONS WHERE AD_MODULE_ID='"
             + _moduleId + "'))) ORDER BY U.INDEX_NAME");
     _stmt_listindexes_noprefix = _connection
-        .prepareStatement("SELECT U.INDEX_NAME, U.UNIQUENESS, UE.COLUMN_EXPRESSION, U.TABLE_OWNER FROM USER_INDEXES U LEFT JOIN USER_IND_EXPRESSIONS UE ON U.INDEX_NAME = UE.INDEX_NAME WHERE U.TABLE_NAME = ? AND (U.INDEX_TYPE = 'NORMAL' OR U.INDEX_TYPE = 'FUNCTION-BASED NORMAL') AND NOT EXISTS (SELECT 1 FROM USER_CONSTRAINTS WHERE TABLE_NAME = U.TABLE_NAME AND INDEX_NAME = U.INDEX_NAME AND CONSTRAINT_TYPE IN ('U', 'P')) AND upper(U.INDEX_NAME) NOT LIKE 'EM_%'  ORDER BY U.INDEX_NAME");
+        .prepareStatement("SELECT U.INDEX_NAME, U.UNIQUENESS, U.TABLE_OWNER FROM USER_INDEXES U WHERE U.TABLE_NAME = ? AND (U.INDEX_TYPE = 'NORMAL' OR U.INDEX_TYPE = 'FUNCTION-BASED NORMAL') AND NOT EXISTS (SELECT 1 FROM USER_CONSTRAINTS WHERE TABLE_NAME = U.TABLE_NAME AND INDEX_NAME = U.INDEX_NAME AND CONSTRAINT_TYPE IN ('U', 'P')) AND upper(U.INDEX_NAME) NOT LIKE 'EM_%'  ORDER BY U.INDEX_NAME");
     _stmt_indexcolumns = _connection
-        .prepareStatement("SELECT COLUMN_NAME FROM USER_IND_COLUMNS WHERE INDEX_NAME = ? ORDER BY COLUMN_POSITION");
+        .prepareStatement(" SELECT column_name, column_expression FROM USER_IND_COLUMNS i left join USER_IND_EXPRESSIONS e on e.index_name = i.index_name and i.column_position = e.column_position\n"
+            + " WHERE i.INDEX_NAME = ? ORDER BY i.COLUMN_POSITION");
 
     _stmt_listuniques = _connection
         .prepareStatement("SELECT CONSTRAINT_NAME FROM USER_CONSTRAINTS WHERE CONSTRAINT_TYPE = 'U' AND TABLE_NAME = ? ORDER BY CONSTRAINT_NAME");
@@ -328,23 +329,24 @@
     inx.setName(indexName);
     inx.setUnique(translateUniqueness(rs.getString(2)));
 
-    // The index expression will be defined only for function based indexes
-    final String indexExpression = rs.getString(3);
-    final String databaseOwner = rs.getString(4);
+    final String databaseOwner = rs.getString(3);
+
     _stmt_indexcolumns.setString(1, indexRealName);
     fillList(_stmt_indexcolumns, new RowFiller() {
       public void fillRow(ResultSet r) throws SQLException {
         String columnName = r.getString(1);
         IndexColumn inxcol = null;
-        if (indexExpression != null && !indexExpression.isEmpty()
-            && columnName.startsWith(VIRTUAL_COLUMN_PREFIX)) {
-          // The name of function based index columns needs to be translated from the name of the
+        if (columnName.startsWith(VIRTUAL_COLUMN_PREFIX)) {
+          // The name of function base index columns needs to be translated from the name of the
           // virtual column created by Oracle to the name of the column in its table
+          final String indexExpression = r.getString(2);
+
           inxcol = getFunctionBasedIndexColumn(indexExpression, databaseOwner);
         } else {
           inxcol = new IndexColumn();
           inxcol.setName(columnName);
         }
+
         String operatorClass = getIndexOperatorClass(indexName, inxcol.getName());
         if (operatorClass != null && !operatorClass.isEmpty()) {
           if (DBSMContants.CONTAINS_SEARCH.equals(operatorClass)) {
--- a/src/org/apache/ddlutils/platform/postgresql/PostgreSqlModelLoader.java	Mon Mar 13 14:08:29 2017 +0100
+++ b/src/org/apache/ddlutils/platform/postgresql/PostgreSqlModelLoader.java	Wed Mar 15 13:35:00 2017 +0100
@@ -1,6 +1,6 @@
 /*
  ************************************************************************************
- * Copyright (C) 2001-2016 Openbravo S.L.U.
+ * Copyright (C) 2001-2017 Openbravo S.L.U.
  * Licensed under the Apache Software License version 2.0
  * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
  * Unless required by applicable law or agreed to  in writing,  software  distributed
@@ -67,7 +67,7 @@
 
   protected Map<Integer, Integer> _paramtypes = new HashMap<Integer, Integer>();
 
-  private static final String FUNCTION_BASED_COLUMN_INDEX_POSITION = "0";
+  private static final int FUNCTION_BASED_COLUMN_INDEX_POSITION = 0;
 
   /** Creates a new instance of PostgreSqlModelLoader */
   public PostgreSqlModelLoader() {
@@ -251,7 +251,8 @@
             + " WHERE pc.contype='f' and pc.conrelid= pc1.oid and pc.conname = ? and pa1.attrelid = pc1.oid and pa1.attnum = ANY(pc.conkey)"
             + " and pc.confrelid = pc2.oid and pa2.attrelid = pc2.oid and pa2.attnum = ANY(pc.confkey)");
 
-    sql = "SELECT PG_CLASS.RELNAME, CASE PG_INDEX.indisunique WHEN true THEN 'UNIQUE' ELSE 'NONUNIQUE' END, PG_GET_EXPR(PG_INDEX.indexprs,PG_INDEX.indrelid, true), PG_INDEX.indclass, PG_GET_EXPR(PG_INDEX.indpred,PG_INDEX.indrelid,true)"
+    sql = "SELECT PG_CLASS.RELNAME, CASE PG_INDEX.indisunique WHEN true THEN 'UNIQUE' ELSE 'NONUNIQUE' END, "
+        + " PG_INDEX.indclass, PG_GET_EXPR(PG_INDEX.indpred,PG_INDEX.indrelid,true)"
         + " FROM PG_INDEX, PG_CLASS, PG_CLASS PG_CLASS1, PG_NAMESPACE"
         + " WHERE PG_INDEX.indexrelid = PG_CLASS.OID"
         + " AND PG_INDEX.indrelid = PG_CLASS1.OID"
@@ -263,6 +264,7 @@
         + " AND PG_CLASS.RELNAME NOT IN (SELECT pg_constraint.conname::text "
         + "    FROM pg_constraint JOIN pg_class ON pg_class.oid = pg_constraint.conrelid"
         + "    WHERE pg_constraint.contype = 'u')";
+
     _stmt_listindexes = _connection.prepareStatement(sql + " ORDER BY UPPER(PG_CLASS.RELNAME)");
     _stmt_listindexes_noprefix = _connection.prepareStatement(sql
         + " AND upper(PG_CLASS.RELNAME) NOT LIKE 'EM\\\\_%'" + " ORDER BY UPPER(PG_CLASS.RELNAME)");
@@ -273,15 +275,13 @@
             + "\\\\_%' OR (UPPER(PG_CLASS.RELNAME::TEXT)||UPPER(PG_CLASS1.RELNAME::TEXT) IN (SELECT upper(NAME1)||UPPER(NAME2) FROM AD_EXCEPTIONS WHERE AD_MODULE_ID='"
             + _moduleId + "')))" + " ORDER BY UPPER(PG_CLASS.RELNAME)");
 
+    // 1. index def: can be column name for standard indexes, or expression if function based
+    // 2. key: 0-> identifies function based index, any other value -> standard column index
     _stmt_indexcolumns = _connection
-        .prepareStatement("SELECT upper(pg_attribute.attname::text), pg_attribute.attnum, array_to_string(pg_index.indkey,',') "
-            + "FROM pg_index, pg_class, pg_namespace, pg_attribute"
-            + " WHERE pg_index.indexrelid = pg_class.oid"
-            + " AND pg_attribute.attrelid = pg_index.indrelid"
-            + " AND pg_attribute.attnum = ANY (indkey)"
-            + " AND pg_class.relnamespace = pg_namespace.oid"
-            + " AND pg_namespace.nspname = current_schema() AND pg_class.relname = ?"
-            + " ORDER BY pg_attribute.attnum");
+        .prepareStatement("select pg_get_indexdef(c.oid, an, true), indkey[an-1]\n"
+            + "from pg_index i,pg_class c, generate_series(1, i.indnatts) an\n"
+            + "where c.oid = i.indexrelid and c.relname = ?\n" //
+            + "order by an");
 
     sql = "SELECT pg_constraint.conname"
         + " FROM pg_constraint JOIN pg_class ON pg_class.oid = pg_constraint.conrelid"
@@ -929,7 +929,7 @@
   protected Index readIndex(ResultSet rs) throws SQLException {
     String indexRealName = rs.getString(1);
     String indexName = indexRealName.toUpperCase();
-    String indexWhereClause = rs.getString(5);
+    String indexWhereClause = rs.getString(4);
 
     final Index inx = new Index();
 
@@ -939,67 +939,21 @@
       inx.setWhereClause(transformIndexExpression(indexWhereClause));
     }
 
-    /*
-     * Note: only element 0 of this list will ever be used.
-     * 
-     * Contains a ','-separated strings of the indices of the index-column into the positions of the
-     * corresponding columns in the table. Example if an index has the fourth,second,third column of
-     * a table this contains the string "4,2,3"
-     */
-    final ArrayList<String> apositions = new ArrayList<String>();
-
-    /*
-     * This maps the columnIndex of an index-column (in the list of columns of the table) to its
-     * IndexColumn object. Example: If an IndexColumn is defined on the second column in the table
-     * this map is <2,IndexColumn>
-     */
-    final HashMap<String, IndexColumn> colMap = new HashMap<String, IndexColumn>();
-
     _stmt_indexcolumns.setString(1, indexRealName);
     fillList(_stmt_indexcolumns, new RowFiller() {
       public void fillRow(ResultSet r) throws SQLException {
-        apositions.add(r.getString(3));
-        IndexColumn inxcol = new IndexColumn();
-        inxcol.setName(r.getString(1));
-        colMap.put(r.getString(2), inxcol);
+        IndexColumn indexColumn;
+        if (r.getInt(2) == FUNCTION_BASED_COLUMN_INDEX_POSITION) {
+          indexColumn = getIndexColumnFromExpression(r.getString(1));
+        } else {
+          indexColumn = new IndexColumn(r.getString(1).toUpperCase());
+        }
+        inx.addColumn(indexColumn);
       }
     });
 
-    String indexExpression = rs.getString(3);
-    if (indexExpression != null && !indexExpression.isEmpty()) {
-      IndexColumn indexColumn = getIndexColumnFromExpression(indexExpression);
-      // colMap contains the index column where functions are not applied
-      // functionBasedIndexColumnList contains the index column where functions are applied
-      if (colMap.isEmpty()) {
-        // there are only function based index columns, no need to merge them with non-function
-        // based columns
-        inx.addColumn(indexColumn);
-      } else {
-        // if colMap is not empty we can be sure that apositions is not empty either
-        // all the values of apositions contains the same values, so we just take the first one
-        for (String pos : (apositions.get(0).split(","))) {
-          // if the position is FUNCTION_BASED_COLUMN_INDEX_POSITION, that means that the next
-          // column should be a function based one, use the next one
-          if (FUNCTION_BASED_COLUMN_INDEX_POSITION.equals(pos)) {
-            inx.addColumn(indexColumn);
-          } else {
-            inx.addColumn(colMap.get(pos));
-          }
-        }
-      }
-    } else {
-      /*
-       * Re-order the IndexColumn into the correct order (based on index-definition) as they are
-       * read without ordering them from the metadata
-       */
-      if (apositions.size() > 0) {
-        for (String pos : (apositions.get(0).split(","))) {
-          inx.addColumn(colMap.get(pos));
-        }
-      }
-    }
     // Obtain the operatorClassOids of each index column
-    String[] operatorClassOids = rs.getString(4).split(" ");
+    String[] operatorClassOids = rs.getString(3).split(" ");
     int i = 0;
     int trgmOperators = 0;
     for (IndexColumn indexColumn : inx.getColumns()) {
@@ -1057,6 +1011,7 @@
   private String transformIndexExpression(String indexExpression) {
     String transformedIndexExpression = removeCastExpressions(indexExpression);
     transformedIndexExpression = transformUnquotedPartOfExpression(transformedIndexExpression);
+    transformedIndexExpression = removeParentheses(transformedIndexExpression);
     return transformedIndexExpression;
   }
 
@@ -1070,6 +1025,18 @@
         replacementGroup);
   }
 
+  /**
+   * In some occasions, PG adds extra parenthesis to beginning and end of the expression, remove
+   * them as they will be always added when the index is created. In this way, the expression gets
+   * identically exported in ORA and PG.
+   */
+  private String removeParentheses(String expr) {
+    if (expr.startsWith("(") && expr.endsWith(")")) {
+      return expr.substring(1, expr.length() - 1);
+    }
+    return expr;
+  }
+
   private String replaceRegularExpressionMatchings(String string, String regExp, int groupIndex) {
     Pattern pattern = Pattern.compile(regExp);
     Matcher matcher = pattern.matcher(string);