[Mondrian] RE: Eigenbase perforce change 12593 for review

[Julian Hyde]

Sherman,

I think you misunderstand how pluggable dialects work. They CAN be created
independently of database products. (Until your change, that is.)

There is a problem with instantiating MySQL & Infobright dialects because
the databases look so similar. However, there is a much simpler fix that
doesn't break the fundamentals of how dialect factories work.

Can you please back out this change and I will apply the fix.

Julian

> -----Original Message-----
> From: Sherman Wood [mailto:sgwood at users.sf.net] 
> Sent: Sunday, April 12, 2009 1:08 PM
> To: Ajit Joglekar; Aaron Phillips; Andreas Voss; Bart Pappyn; 
> Ezequiel Cuellar; Julian Hyde; John V. Sichi; Mat Lowery; 
> Matt Campbell; Rushan Chen; Robin Tharappel; Will Gorman; Zelaine Fong
> Subject: Eigenbase perforce change 12593 for review
> 
> http://p4web.eigenbase.org/@md=d&c=6PU@//12593?ac=10
> 
> Change 12593 by sgwood at sgwood_perforce.eigenbase.org_1666_1 
> on 2009/04/12 13:07:51
> 
> 	MONDRIAN: Fix for MONDRIAN-528 - MySQL incorrectly 
> identified as using Infobright Dialect.
> 	Included notes in Javadoc for implementing Dialects
> 
> Affected files ...
> 
> ... 
> //open/mondrian/src/main/mondrian/spi/impl/InfobrightDialect.j
> ava#3 edit
> ... 
> //open/mondrian/src/main/mondrian/spi/impl/JdbcDialectFactory.
> java#2 edit
> ... 
> //open/mondrian/src/main/mondrian/spi/impl/JdbcDialectImpl.java#8 edit
> ... 
> //open/mondrian/src/main/mondrian/spi/impl/MySqlDialect.java#5 edit
> 
> Differences ...
> 
> ==== 
> //open/mondrian/src/main/mondrian/spi/impl/InfobrightDialect.j
> ava#3 (ktext) ====
> 
> 18c18
> <  * @version $Id: 
> //open/mondrian/src/main/mondrian/spi/impl/InfobrightDialect.java#2 $
> ---
> >  * @version $Id: 
> //open/mondrian/src/main/mondrian/spi/impl/InfobrightDialect.java#3 $
> 26,28c26
> <             // While we're choosing dialects, this still 
> looks like a MySQL
> <             // connection.
> <             DatabaseProduct.MYSQL);
> ---
> >             DatabaseProduct.INFOBRIGHT);
> 39,42d36
> <     public DatabaseProduct getDatabaseProduct() {
> <         return DatabaseProduct.INFOBRIGHT;
> <     }
> < 
> 
> ==== 
> //open/mondrian/src/main/mondrian/spi/impl/JdbcDialectFactory.
> java#2 (ktext) ====
> 
> 112a113,123
> >     /**
> >      * Generate a valid dialect for the connection, or null if
> >      * the connection is not for the given DatabaseProduct.
> >      *
> >      * Instantiates a Dialect and tests it. Dialects may 
> run queries and other
> >      * operations to determine the database product behind 
> the connection.
> >      *
> >      * @param dataSource JDBC data source, if null need connection
> >      * @param connection JDBC connection, if null nee data source
> >      * @return Dialect, or null if factory cannot create 
> suitable dialect
> >      */
> 121,136d131
> <         if (acceptsConnection(connection)) {
> <             try {
> <                 return constructor.newInstance(connection);
> <             } catch (InstantiationException e) {
> <                 throw Util.newError(
> <                     e, "Error while instantiating dialect");
> <             } catch (IllegalAccessException e) {
> <                 throw Util.newError(
> <                     e, "Error while instantiating dialect");
> <             } catch (InvocationTargetException e) {
> <                 throw Util.newError(
> <                     e, "Error while instantiating dialect");
> <             }
> <         }
> <         return null;
> <     }
> 138,144c133
> <     /**
> <      * Returns whether this dialect is suitable for the 
> given connection.
> <      *
> <      * @param connection Connection
> <      * @return Whether suitable
> <      */
> <     private boolean acceptsConnection(Connection connection) {
> ---
> >         Dialect newDialect = null;
> 146,152c135,142
> <             final DatabaseMetaData metaData = 
> connection.getMetaData();
> <             final String productName = 
> metaData.getDatabaseProductName();
> <             final String productVersion = 
> metaData.getDatabaseProductVersion();
> <             final Dialect.DatabaseProduct product =
> <                 JdbcDialectImpl.getProduct(productName, 
> productVersion);
> <             return product == this.databaseProduct;
> <         } catch (SQLException e) {
> ---
> >             newDialect = constructor.newInstance(connection);
> >         } catch (InstantiationException e) {
> >             throw Util.newError(
> >                 e, "Error while instantiating dialect");
> >         } catch (IllegalAccessException e) {
> >             throw Util.newError(
> >                 e, "Error while instantiating dialect");
> >         } catch (InvocationTargetException e) {
> 155a146,152
> > 
> >         if (newDialect == null ||
> >                 newDialect.getDatabaseProduct() != 
> this.databaseProduct) {
> >             return null;
> >         } else {
> >             return newDialect;
> >         }
> 
> ==== 
> //open/mondrian/src/main/mondrian/spi/impl/JdbcDialectImpl.jav
> a#8 (ktext) ====
> 
> 30a31,40
> >  * <p>Implementing a Dialect will include changing the 
> getProduct method in this
> >  * class and changing the list of Dialects in the {@link 
> Dialect} interface. If
> >  * dialect specific SQL is required in the schema, the 
> Mondrian schema will need
> >  * to be updated.</p>
> >  *
> >  * <p>Note that subclasses of JdbcDialectImpl will be 
> instantiated with JDBC
> >  * connections that are not of the actual database product 
> the subclass
> >  * supports. This is done as part of determining the 
> correct dialect for the
> >  * connection.</p>
> >  *
> 32c42
> <  * @version $Id: 
> //open/mondrian/src/main/mondrian/spi/impl/JdbcDialectImpl.java#7 $
> ---
> >  * @version $Id: 
> //open/mondrian/src/main/mondrian/spi/impl/JdbcDialectImpl.java#8 $
> 
> ==== 
> //open/mondrian/src/main/mondrian/spi/impl/MySqlDialect.java#5
>  (ktext) ====
> 
> 14d13
> < import javax.sql.DataSource;
> 22c21
> <  * @version $Id: 
> //open/mondrian/src/main/mondrian/spi/impl/MySqlDialect.java#4 $
> ---
> >  * @version $Id: 
> //open/mondrian/src/main/mondrian/spi/impl/MySqlDialect.java#5 $
> 30,50c29
> <             DatabaseProduct.MYSQL)
> <         {
> <             @Override
> <             public Dialect createDialect(
> <                 DataSource dataSource,
> <                 Connection connection)
> <             {
> <                 final Dialect dialect =
> <                     super.createDialect(
> <                         dataSource, connection);
> <                 // Infobright looks a lot like MySQL. If 
> this is an Infobright
> <                 // connection, yield to the Infobright dialect.
> <                 if (dialect != null &&
> <                     dialect instanceof MySqlDialect &&
> <                     ((MySqlDialect) dialect).getDatabaseProduct() ==
> <                     DatabaseProduct.INFOBRIGHT) {
> <                     return null;
> <                 }
> <                 return dialect;
> <             }
> <         };
> ---
> >             DatabaseProduct.MYSQL);
> 75a55,56
> >             String productName =
> >                 databaseMetaData.getDatabaseProductName();
> 77a59,64
> > 
> >             if (JdbcDialectImpl.getProduct(productName, 
> productVersion) !=
> >                     DatabaseProduct.MYSQL) {
> >                 return false;
> >             }
> > 
> 
>