[PATCH] [REVIEW:3-5] PostgreSQL getColumns()

getColumns() is a XDatabaseMetadata interface function to get the list
of columns of a table and info on these columns.

The PostgreSQL-SDBC implementation in 3.5.0 has a couple of bugs fixed
by the attached patch.

 - If the table previously had a column that has since been dropped
   (removed), the numbering of the columns has a hole, and in some
   circumstances the dropped column is still shown by getColumns().

   "Still shown" fixed by

+                   "AND NOT pg_attribute.attisdropped "

   which is the boolean in the PostgreSQL internals that says "this
   column has been dropped, don't show it anymore".

   Numbering had a hole was because it was using "attnum", the
   internal PostgreSQL numbering of columns. But this internal
   numbering is not contiguous, as the number of dropped columns are
   *not* recycled. This is fixed by:

   * Removing attnum from the query we send to PostgreSQL.
   * Adapt column numbers (shifted by one) every time a column (after
     attnum) is read.
   * Generate our own numbering and put that in the result.

   Note that "#invalid#" cannot be a table or schema name: character
   '#' is not allowed.


 - Entries were sorted by the concatenation (?) of schema name, table
   name, column name. In rare cases, this could lead to wrong order,
   and is slower anyway. Example:

   library, book, bookID
   library, bookShelf, bookShelfID
   library, book, XID

   is sorted in this order by "concatenation", but in this (correct) order
   when sorting by column:

   library, book, bookID
   library, book, XID
   library, bookShelf, bookShelfID


   "||" is the SQL string concatenation operator.


I don't have a smoking gun fdo# of a "point and click user"-visible
bug of this, partially because our internal code is suspicious about
some of this data... See lcl_sanitizeColumnDescs in
connectivity/source/commontools/TTableHelper.cxx.

But I consider Base also as a programming platform,
user code (scripts) is allowed to call any function in
XDatabaseMetadata with any arguments, and giving a wrong result (data)
back is a bug in itself.

So I'd like to have this all fixed in libreoffice-3-5, too.

-- 
Lionel

From 710d460eb9f30f0ba33b3ea6ac6d43e320144513 Mon Sep 17 00:00:00 2001
From: Lionel Elie Mamane <lionel@mamane.lu>
Date: Wed, 1 Feb 2012 17:18:48 +0100
Subject: [PATCH 2/2] postgres getColumns: skip dropped columns, make
 ORDINAL_POSITION consecutive

---
 .../drivers/postgresql/pq_databasemetadata.cxx     |   58 +++++++++++++-------
 1 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/connectivity/source/drivers/postgresql/pq_databasemetadata.cxx 
b/connectivity/source/drivers/postgresql/pq_databasemetadata.cxx
index f359eaa..b3fedf6 100644
--- a/connectivity/source/drivers/postgresql/pq_databasemetadata.cxx
+++ b/connectivity/source/drivers/postgresql/pq_databasemetadata.cxx
@@ -1530,7 +1532,9 @@ static void columnMetaData2DatabaseTypeDescription(
     }
 
     // ignore catalog, as a single pq connection
-    // does not support multiple catalogs eitherway
+    // does not support multiple catalogs anyway
+    // We don't use information_schema.columns because it contains
+    // only the columns the current user has any privilege over.
 
     //  1. TABLE_CAT string => table catalog (may be NULL)
     //               => not supported
@@ -1561,9 +1565,7 @@ static void columnMetaData2DatabaseTypeDescription(
     //                      NULLABLE_UNKNOWN - nullability unknown
     //               => pg_attribute.attnotnull
     //  12. REMARKS string => comment describing column (may be NULL )
-    //               => Don't know, there does not seem to exist something like
-    //                  that in postgres
-    //               LEM TODO: comments exist, find how to retrieve them easily
+    //               => pg_description.description
     //  13. COLUMN_DEF string => default value (may be NULL)
     //               => pg_type.typdefault
     //  14. SQL_DATA_TYPE long => unused
@@ -1591,11 +1593,10 @@ static void columnMetaData2DatabaseTypeDescription(
             "pg_attribute.atttypmod, "       // 5
             "pg_attribute.attnotnull, "      // 6
             "pg_type.typdefault, "           // 7
-            "pg_attribute.attnum, "          // 8
-            "pg_type.typtype, "              // 9
-            "pg_attrdef.adsrc, "             // 10
-            "pg_description.description, "    // 11
-            "pg_type.typbasetype "           // 12
+            "pg_type.typtype, "              // 8
+            "pg_attrdef.adsrc, "             // 9
+            "pg_description.description, "   // 10
+            "pg_type.typbasetype "           // 11
             "FROM pg_class, "
                  "pg_attribute LEFT JOIN pg_attrdef ON pg_attribute.attrelid = pg_attrdef.adrelid 
AND pg_attribute.attnum = pg_attrdef.adnum "
                               "LEFT JOIN pg_description ON pg_attribute.attrelid = 
pg_description.objoid AND pg_attribute.attnum=pg_description.objsubid,"
@@ -1603,10 +1604,11 @@ static void columnMetaData2DatabaseTypeDescription(
             "WHERE pg_attribute.attrelid = pg_class.oid "
                    "AND pg_attribute.atttypid = pg_type.oid "
                    "AND pg_class.relnamespace = pg_namespace.oid "
+                   "AND NOT pg_attribute.attisdropped "
                    "AND pg_namespace.nspname LIKE ? "
                    "AND pg_class.relname LIKE ? "
                    "AND pg_attribute.attname LIKE ? "
-            "ORDER BY pg_namespace.nspname || pg_class.relname || pg_attribute.attnum"
+            "ORDER BY pg_namespace.nspname, pg_class.relname, pg_attribute.attnum"
             ) );
 
     Reference< XParameters > parameters( statement, UNO_QUERY_THROW );
@@ -1632,31 +1634,47 @@ static void columnMetaData2DatabaseTypeDescription(
     Reference< XStatement > domainTypeStmt = m_origin->createStatement();
     columnMetaData2DatabaseTypeDescription( domainMap, rs, domainTypeStmt );
 
+    unsigned int colNum;
+    OUString sSchema( ASCII_STR("#invalid#") );
+    OUString sTable(  ASCII_STR("#invalid#") );
+
     while( rs->next() )
     {
         OUString columnName = xRow->getString(3);
         if( m_pSettings->showSystemColumns || ! isSystemColumn( columnName ) )
         {
+            OUString sNewSchema( xRow->getString(1) );
+            OUString sNewTable(  xRow->getString(2) );
+            if ( sNewSchema != sSchema || sNewTable != sTable )
+            {
+                colNum = 1;
+                sSchema = sNewSchema;
+                sTable = sNewTable;
+            }
+            else
+                ++colNum;
             sal_Int32 precision, scale, type;
             Sequence< Any > row( 18 );
             row[0] <<= m_pSettings->catalog;
-            row[1] <<= xRow->getString(1);  //
-            row[2] <<= xRow->getString(2);
+            row[1] <<= sNewSchema;
+            row[2] <<= sNewTable;
             row[3] <<= columnName;
-            if( xRow->getString(9).equalsAscii( "d" ) )
+            if( xRow->getString(8).equalsAscii( "d" ) )
             {
-                DatabaseTypeDescription desc( domainMap[xRow->getInt(12)] );
+                DatabaseTypeDescription desc( domainMap[xRow->getInt(11)] );
                 type = typeNameToDataType( desc.typeName, desc.typeType );
             }
             else
             {
-                type = typeNameToDataType( xRow->getString(4), xRow->getString(9) );
+                type = typeNameToDataType( xRow->getString(4), xRow->getString(8) );
             }
             extractPrecisionAndScale( type, xRow->getInt(5) , &precision, &scale );
             row[4] <<= type;
             row[5] <<= xRow->getString(4);
             row[6] <<= precision;
+            // row[7] BUFFER_LENGTH not used
             row[8] <<= scale;
+            // row[9] RADIX TODO
             if( xRow->getBoolean( 6 ) && ! isSystemColumn(xRow->getString(3)) )
             {
                 row[10] <<= OUString::valueOf(com::sun::star::sdbc::ColumnValue::NO_NULLS);
@@ -1668,13 +1686,13 @@ static void columnMetaData2DatabaseTypeDescription(
                 row[17] <<= statics.YES;
             }
 
-            row[11] <<= xRow->getString( 11 ); // comment
-            row[12] <<= xRow->getString(10); // COLUMN_DEF = pg_type.typdefault
+            row[11] <<= xRow->getString( 10 ); // comment
+            row[12] <<= xRow->getString( 9 ); // COLUMN_DEF = pg_type.typdefault
+            // row[13] SQL_DATA_TYPE    not used
+            // row[14] SQL_DATETIME_SUB not used
             row[15] <<= precision;
-            row[16] <<= xRow->getString(8) ;
+            row[16] <<= colNum ;
 
-            // no description in postgresql AFAIK
             vec.push_back( row );
         }
     }
-- 
1.7.7.3