[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Mikael Kopteff commented on HHH-2933: ------------------------------------- Actually adding more key/value pairs to the MAPPERS attribute would be even better, so the original names could not be changed, but new database names introduced. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Steve Ebersole commented on HHH-2933: ------------------------------------- Tomoto, I appreciate the work you put into this and I largely like the result. The one thing I would change would be to not have Dialects themselves involved at all in registration. The reason being that many Dialects could register against a single set of connection metadata. Take an Oracle 8i database as an example... Just in terms of Dialects Hibernate ships, we as end-users could conceivably want to use Oracle8iDialect, Oracle9iDialect Oracle10gDialect, DataDirectOracle9Dialect, etc depending on our requirements. The "finder" or "resolver" approach is better I think because it would allow explicit control over this facet. Hibernate could provide a standard one that does the same basic thing we currently do with DialectFactory#MAPPERS. This would also act as the "fallback" resolver, allowing custom-registered resolvers a chance to do resolution first. Also, in terms of impl, I think it would be better to pass along the Connection for resolution rather than individual pieces of metadata information from the connection. So I'd prefer: public interface DialectResolver { /** * Determine the dialect to use based on the configured connection. Implementations should * return null to indicate that they have no knowledge of which dialect to use for the given * connection. * * @param connection The connection * * @return The dialect we want used for this connection, or null if we cannot make such a determination. */ public Dialect resolveDialect(Connection connection); } -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Tomoto Shimizu Washio commented on HHH-2933: -------------------------------------------- Regarding my question (1), it seems passing Connection would be OK... There would be no point to hate it because DatabaseMetaData provided some methods that seemed to access to the database to get the information, which would mean giving a virtual permission to DialectResolver to access to the database anyway. Let me give it a shot. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Steve Ebersole updated HHH-2933: -------------------------------- Assignee: Steve Ebersole Description: Introducing DialectResolver! Much better approach to resolving JDBC Connection information to Dialect allowing users to define custom behavior was: Currently the DialectFactory has the static attribute MAPPERS, that hold the dialects. If the dialect is not found, Hibernate will throw an exception. There is already an TODO tag, set for this.So, could this be possibly done, so that the values really are a) in a properties file or b)additions could be made to the map programmaticaly by users. This would be more flexible for advanced users, that work with experimental jdbc drivers(that don't return a standard name for the database). The current trunk version of DialectFactory: // TODO : this is the stuff it'd be nice to move to a properties file or some other easily user-editable place private static final Map MAPPERS = new HashMap(); static { MAPPERS.put( "HSQL Database Engine", new VersionInsensitiveMapper( "org.hibernate.dialect.HSQLDialect" ) ); MAPPERS.put( "H2", new VersionInsensitiveMapper( "org.hibernate.dialect.H2Dialect" ) ); MAPPERS.put( "MySQL", new VersionInsensitiveMapper( "org.hibernate.dialect.MySQLDialect" ) ); etc..... Fix Version/s: 3.4 3.3.x 3.2.x Issue Type: Patch (was: Improvement) Summary: externalize automatic resolution of Dialect (Tomoto Shimizu Washio) (was: DialectFactory dialects should be in a properties file) -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Steve Ebersole commented on HHH-2933: ------------------------------------- to answer your specific questions: 1) You asked about the current methods on DialectFactory. I don't not consider DialectFactory as part of the public API. I'll leave the methods as deprecated for 3.2 and 3.3, but remove them on trunk (3.4) 2) I could have been convinced to use passing in DatabaseMetaData instead of Connection. But it sounds like you convinced yourself to use Connection... 3) Yes, I prefer that DialectResolver return the Dialect instance. The reason being a longer term goal of possibly allowing Dialects to be non-static and therefore allowing Dialect Resolver to "configure" the Dialect it returns based on the Connection. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Tomoto Shimizu Washio commented on HHH-2933: -------------------------------------------- Thank you very much for working on this issue as well as answering to my questions! I think passing DatabaseMetaData would be still fine. It could eliminate connection.getMetaData() invocation and save one line from each DialectResolver implementation. If you prefer this (actually I do), please change the interface. As for SQLException... I noticed that each method on DatabaseMetaData could throw SQLException when the JDBC driver did not support the requested metadata. This means it would be difficult for DialectResolver to tell if the error was really caused by the connection problem (the process should be stopped) or just this JDBC driver did not support the information that the resolver required (this resolver does not handle this connection). Said that, if it were important to distinguish those two cases, I think the following strategy should work: have a contract where "it is DialectResolver's responsibility to avoid to access to unsupported metadata so that throwing SQLException would always mean the connection problem that should stop the determination process". Then, let DialectResolverSet not suppress SQLException. Now SQLException thrown by a resolver is caught by DialectFactory and the determination process is stopped immediately. Also, I would rather pass the database name and the database major version to the resolver along with the connection (or metadata) to ensure that the resolver could at least determine if the JDBC driver is known and which metadata are supported on this JDBC driver before accessing to any metadata. And, although we might think of allowing DialectResolver to throw only HibernateException but not SQLException, I doubt it would be really necessary. Given SQLException meant connection errors as discussed above, the handling process of this exception would not be resolver-dependent and it would be enough to have one handling process in DialectFactory layer. Hope it helps, and please let me know if I could be of help. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Steve Ebersole commented on HHH-2933: ------------------------------------- DatabaseMetaData has a method getConnection() so personally I think its '6 in one, half dozen in the other'. But I think having DatabaseMetaData as the param type conveys the intent more clearly, so lets go with that. With regards to SQLException, what I mean is that given druthers I would only allow java.sql.SQLNonTransientConnectionException if we were using Java 6. But of course that is not the case. As it is, allowing throwing of the generic SQLException does not convey the intent. Sadly I'd have to say that imo most developers would not even check the javadocs here; they'd just see 'SQLException' allowed in the throws clause and not handle it at all. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Tomoto Shimizu Washio commented on HHH-2933: -------------------------------------------- Thank you to elaborate. I understand your intention. DatabaseMetaData in the parameter is great, yeah! I agree it would make sense avoiding SQLException to clearly show the intent and enforce the implementors to do what they should do. I was a little afraid that it might impose unnecessary exception handling on the resolvers, but now I think it would not be much a problem because they could still extend SimpleDialectResolver in order not to be bothered by SQLException if they needed only the very basic information such as database name. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Tomoto Shimizu Washio commented on HHH-2933: -------------------------------------------- Thank you, Steve. I will take a look at it today and respond probably in six hours. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Steve Ebersole commented on HHH-2933: ------------------------------------- 1) To make it explicit I actually added the 'else' clause to 'if ( useJdbcMetadata )' 2) Done 3) Given the current set up its not such a big deal. But its better to avoid, so I moved DialectFactory to o.g.dialect.resolver which alleviates the cycle. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Steve Ebersole updated HHH-2933: -------------------------------- Summary: allow custom resolution of Dialect (Tomoto Shimizu Washio) (was: externalize automatic resolution of Dialect (Tomoto Shimizu Washio)) -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-2933?page=c... ] Tomoto Shimizu Washio commented on HHH-2933: -------------------------------------------- I have taken a look at the code in svn and it looks fine. Thank you! -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira