[hibernate-issues] [Hibernate-JIRA] Commented: (HHH-5966) Finish up loose ends for overriding a SqlTypeDescriptor

[ http://opensource.atlassian.com/projects/hibernate/browse/HHH-5966?page=c... ] Steve Ebersole commented on HHH-5966: ------------------------------------- {noformat} [04/01/11 15:10] <sebersole> gbadner: question for you wrt sql type remapping [04/01/11 15:10] <sebersole> why are you only remapping certain instances? [04/01/11 15:10] <sebersole> or put better [04/01/11 15:11] <sebersole> why do you only remap based on instance checking [04/01/11 15:11] <gbadner> sebersole, just commented on that pull request [04/01/11 15:11] <sebersole> as opposed to sql code checking [04/01/11 15:11] <gbadner> just answered that [04/01/11 15:11] <sebersole> pull request? [04/01/11 15:11] <gbadner> https://github.com/hibernate/hibernate-core/pull/43 [04/01/11 15:13] <sebersole> gbadner: well i guess it all boils down to how users implement "custom types" [04/01/11 15:13] <gbadner> yeah [04/01/11 15:13] <sebersole> if they use UserType they will never "tie in" with the remapping... [04/01/11 15:14] <gbadner> no? [04/01/11 15:14] <gbadner> it doesn't use a standard SqlTypeDescriptor? [04/01/11 15:14] <smarlow> sebersole: https://github.com/scottmarlow/hibernate-core/commit/b0444d9c1c37d21658d4... has part of it [04/01/11 15:14] <jbossbot> git [hibernate-core] b0444d9.. Scott Marlow HHH-6073 Dialects shouldn't use TCCL (revised to leave DerbyDialect alone) [04/01/11 15:14] <jbossbot> jira [HHH-6073] Dialects cannot use the Thread Context ClassLoader with AS7, please change to use the [Open (Unresolved) Task, Blocker, Unassigned] http://opensource.atlassian.com/projects/hibernate/browse/HHH-6073 [04/01/11 15:15] <sebersole> it does not expose the notion of SqlTypeDescriptor/JavaTypeDescriptor at all gbadner [04/01/11 15:15] <gbadner> hmmm [04/01/11 15:16] <sebersole> in fact so far I have limited it to single-column variants [04/01/11 15:17] <sebersole> so in the hierarchy it is only org.hibernate.type.AbstractSingleColumnStandardBasicType and "down" [04/01/11 15:18] <sebersole> the "full" approach would require changes so that int[] Type.sqlTypes etc be replaced with SqlTypeDescriptor[] ... [04/01/11 15:18] <gbadner> doesn't UserType define binding/extraction? [04/01/11 15:19] <sebersole> UserType is an interface [04/01/11 15:19] <sebersole> it does not "define" anything [04/01/11 15:19] <sebersole> they can do whatever they want [04/01/11 15:20] <gbadner> oh, ok; well, if they define binding/extraction, why would hibernate override that? [04/01/11 15:20] <smarlow> sebersole: https://github.com/scottmarlow/hibernate-core/commit/c77f933fd96763639486... should be correct now. [04/01/11 15:20] <jbossbot> git [hibernate-core] c77f933.. Scott Marlow HHH-6073 Dialects shouldn't use TCCL (revised to leave DerbyDialect alone) [04/01/11 15:20] <jbossbot> jira [HHH-6073] Dialects cannot use the Thread Context ClassLoader with AS7, please change to use the [Open (Unresolved) Task, Blocker, Unassigned] http://opensource.atlassian.com/projects/hibernate/browse/HHH-6073 [04/01/11 15:20] <sebersole> so you write a UserType that binds Blobs on Postgresql lets say in a way that we know to not work... [04/01/11 15:21] <sebersole> you dont think users would want us to "correct" that? [04/01/11 15:21] <gbadner> well, we provide Lob SQL type descriptors they can use [04/01/11 15:22] <gbadner> if they use a non-default one that works for them, then hibernate won't override it [04/01/11 15:22] <gbadner> only the default LOB SQL descriptors are "standard" SQL type descriptors [04/01/11 15:22] <sebersole> i am not even sure what that means [04/01/11 15:23] <gbadner> "standard" SQL type descriptors are the ones that are used in StandardTypeRegistry [04/01/11 15:24] <sebersole> well but thats an arbitrary distinction [04/01/11 15:24] <sebersole> StandardTypeRegistry is a regsitry for Types [04/01/11 15:24] <sebersole> not SqlTypeDescriptors [04/01/11 15:25] <gbadner> right, but it also defines all the SqlTypeDescriptors used for "standard" types [04/01/11 15:25] <gbadner> the BasicTypeRegistry can be different [04/01/11 15:25] <sebersole> what is the actual use case here/ [04/01/11 15:25] <sebersole> ? [04/01/11 15:26] <sebersole> i mean it feels like fixing a problem that has not come up yet, and so we dont really know the edges of those use cases [04/01/11 15:26] <gbadner> if an application defines it's own SQL type descriptor, Hibernate uses it [04/01/11 15:26] <sebersole> and what is the use case for that? [04/01/11 15:27] <sebersole> to me we either: [04/01/11 15:27] <sebersole> 1) document this, because I dont even have any idea what i am supposed to do here [04/01/11 15:27] <sebersole> 2) wait to see what the actual use cases are [04/01/11 15:27] <sebersole> really, if i understand you correctly [04/01/11 15:28] <sebersole> its not even a matter of them "using" the "standard " SqlTypeDescriptors [04/01/11 15:28] <gbadner> suppose a DBMS has a completely different way of binding/extracting LOBs that the app whant to use [04/01/11 15:28] <sebersole> they also have to know about this whole ability of Dialect remapping [04/01/11 15:28] <gbadner> the sql code would still be Types.BLOB or Types.CLOB [04/01/11 15:29] <sebersole> and why isnt that the standard dialect remap? [04/01/11 15:29] <gbadner> hibernate should not overide the type descriptor; [04/01/11 15:29] <gbadner> maybe it's using a DBMS-specific API [04/01/11 15:30] <sebersole> so then we need option 1 [04/01/11 15:30] <sebersole> document [04/01/11 15:30] <sebersole> and [04/01/11 15:30] <sebersole> that means that this does not work as smoothly as i think you think it does [04/01/11 15:30] <sebersole> because [04/01/11 15:30] <sebersole> for a UserType writer to leverage it [04/01/11 15:30] <sebersole> they *have to* make the dialect remapping calls themselves [04/01/11 15:31] <sebersole> unless maybe they delegate to the equiv basic type calls [04/01/11 15:31] <gbadner> yes, that's what I was thinking [04/01/11 15:32] <sebersole> so then we should document it [04/01/11 15:32] <gbadner> yeah [04/01/11 15:32] <sebersole> becase honestly when i look at it, i see arbitrary decision [04/01/11 15:32] <sebersole> without haveing to ask you [04/01/11 15:32] <sebersole> i mean i get it now [04/01/11 15:32] <gbadner> this is why I wanted to discuss it with you [04/01/11 15:32] <sebersole> ok [04/01/11 15:33] <sebersole> i'm in general not a fan [04/01/11 15:33] <sebersole> even in that case you mention... [04/01/11 15:33] <gbadner> things were too crazy when we were getting ready to release [04/01/11 15:33] <sebersole> either they use the corollary standard basic type [04/01/11 15:33] <sebersole> or use their own binding [04/01/11 15:33] <sebersole> sure [04/01/11 15:34] <gbadner> yeah, that is what is preferred [04/01/11 15:34] <sebersole> to me, for calls that get routed through us we should do what we know is right [04/01/11 15:34] <sebersole> when we know what that is [04/01/11 15:35] <sebersole> i have to think about this some more [04/01/11 15:35] <gbadner> I guess I was thinking more about when an application overrides basic types [04/01/11 15:36] <gbadner> new StringType() { ...} [04/01/11 15:37] <gbadner> or MyNewAndImprovedStringType extends StringType [04/01/11 15:37] <gbadner> not so much UserType [04/01/11 15:37] <sebersole> its still the same SqlTypeDescriptor under then covers [04/01/11 15:37] <sebersole> thats actually the whole reason i wanted it done at that level [04/01/11 15:37] <gbadner> right, unless they define a different SQL type descriptor [04/01/11 15:39] <gbadner> suppose an app wants all strings to be CLOBs [04/01/11 15:39] <sebersole> i mean if you writing an application and have some incredibly better way to extract and bind strings, dont you always want that happening? [04/01/11 15:39] <gbadner> yeah [04/01/11 15:40] <sebersole> well right now, its not [04/01/11 15:40] <sebersole> all strings as Clobs is fine [04/01/11 15:40] <sebersole> 2 different things [04/01/11 15:40] <gbadner> but if the app takes the responsibility for binding/extracting via its own SQL type, I think Hibernate should just use it [04/01/11 15:41] <sebersole> but it can by simply not using the SqlTypeDescriptor to tdo the binding [04/01/11 15:41] <sebersole> nothing says "oh i have to call SqlTypeDescript.getBinder().... for nullSafeSet" [04/01/11 15:42] <sebersole> two different leve;s [04/01/11 15:42] <sebersole> levels [04/01/11 15:42] <gbadner> well, then we should say that they should not use a SqlTypeDescriptor if they don't want the override to happen [04/01/11 15:44] <gbadner> I think it would be surprising if an app supplied it's own SQL type descriptor, and then Hibernate went and used a different one, just because the apps descriptor uses the same SQL code as a built-in thype [04/01/11 15:44] <gbadner> type [04/01/11 15:45] <sebersole> again, its hard to discuss hypotheticals [04/01/11 15:45] <sebersole> what is a concrete use case [04/01/11 15:46] <sebersole> because to me, everyone you mentioned, i think is better served by a custom dialect which provides the needed "vendor api" calls [04/01/11 15:46] <sebersole> as a sqltypedescripto remap [04/01/11 15:47] <sebersole> otherwise, i need to override every single Type in the system that maps to that sql type [04/01/11 15:47] <gbadner> ? [04/01/11 15:48] <sebersole> :( [04/01/11 15:48] <sebersole> remaps work on the level of the jdbc type [04/01/11 15:48] <sebersole> so essentially what you are saying is... [04/01/11 15:48] <sebersole> this database has some special super fast way for us to bind, lets say, VARCHAR values [04/01/11 15:49] <sebersole> well there are potentially 10 different Types that map to Types.VARCHAR [04/01/11 15:49] <sebersole> StringType [04/01/11 15:49] <sebersole> UrlTyope [04/01/11 15:49] <sebersole> UUIDType [04/01/11 15:49] <sebersole> etc [04/01/11 15:49] <sebersole> i need to override every single one of those [04/01/11 15:50] <sebersole> and plug in something with SuperDuperVendorVarcharSqlTypeDescriptor [04/01/11 15:50] <sebersole> or [04/01/11 15:50] <gbadner> no, only the ones that are defined to be a "SuperDuperVendorVarchar" [04/01/11 15:50] <sebersole> its a VARCHAR [04/01/11 15:50] <sebersole> all of those Types are backed by VARCHAR [04/01/11 15:50] <gbadner> yes, I understand [04/01/11 15:50] <gbadner> but [04/01/11 15:52] <gbadner> maybe smallish MaterializedClobs should use setBytes/getBytes [04/01/11 15:52] <gbadner> but large MaterializedClobs should use streams [04/01/11 15:53] <gbadner> oops, meant MaterializedBlobs there [04/01/11 15:53] <sebersole> then make this explicit [04/01/11 15:54] <sebersole> add a SqlTypeDescriptor.isRemappable [04/01/11 15:54] <sebersole> but this magic, "well if its the same instance" thing is confusing [04/01/11 15:54] <gbadner> my thinking was that Dialect didn't have JDBC-impl-specific references [04/01/11 15:54] <sebersole> because its not even the same class [04/01/11 15:54] <sebersole> gbadner: custom dialects do all the time [04/01/11 15:55] <sebersole> its one of the major reasons for writing a custom dialect [04/01/11 15:55] <gbadner> well, ok; I don't know about custom dialects, only the Hibernate ones [04/01/11 15:56] <sebersole> what "JDBC-impl-specific references" have i discussed here? [04/01/11 15:56] <gbadner> I know anyone can create a custom dialect using vendor-specific APIs [04/01/11 15:56] <gbadner> I mentioned about impl-specific APIs for dealing with LOBs [04/01/11 15:57] <gbadner> I thought that kind of thing would be better to define in a custom type than a custom dialect [04/01/11 15:57] <sebersole> why [04/01/11 15:57] <sebersole> its user code in both cases [04/01/11 15:57] <gbadner> true [04/01/11 15:57] <sebersole> would you rather write one or a couple? [04/01/11 15:58] <sebersole> anyway, i have no issue i guess with allowing (or disallowing) sql-type remapping [04/01/11 15:58] <sebersole> if you really think it has value [04/01/11 15:58] <gbadner> well, maybe you've talked me out of it [04/01/11 15:58] <sebersole> but i much prefer that be an explicit contract [04/01/11 15:59] <sebersole> SqlTypeDescriptor.isRemappable() [04/01/11 15:59] <sebersole> and if not, dont remap it [04/01/11 15:59] <sebersole> anyway i can work on that [04/01/11 16:00] <sebersole> i just wanted to understand your concerns there [04/01/11 16:00] <gbadner> here's another use case [04/01/11 16:01] <sebersole> we should document this use cases [04/01/11 16:01] <sebersole> these [04/01/11 16:01] <sebersole> you ok with me pasting this discussion into the jira? [04/01/11 16:01] <gbadner> yeah, that's fine [04/01/11 16:01] <sebersole> although its kinf of longish now i guess [04/01/11 16:02] <gbadner> here's another use case; not sure you'd consider it valid, but here goes [04/01/11 16:03] <gbadner> suppose there are dialects we don't use for testing that require using streams for LOBs [04/01/11 16:04] <gbadner> those dialects will assume the "standard" (default) LOB SQL type descriptors [04/01/11 16:04] <gbadner> but it's wrong [04/01/11 16:04] <gbadner> one way to fix it is to extend the dialect [04/01/11 16:05] <sebersole> k [04/01/11 16:05] <gbadner> fwiu, users do not like to extend dialects, especially if it's because there's a bug in a Hibernate dialect [04/01/11 16:05] <sebersole> k [04/01/11 16:06] <sebersole> not sure i'd call a problem in a non-supported, non-loved dialect a bug, but we'll let that go for this discussion [04/01/11 16:07] <gbadner> the way the can deal with this, for example, is to extend BlobType to use BlobTypeDescriptor.STREAM_BINDING [04/01/11 16:08] <gbadner> BlobTypeDescriptor.STREAM_BINDING is not a standard SQL type descriptor (BlobTypeDescriptor.DEFAULT is the standard one) [04/01/11 16:09] <gbadner> Hibernate will go ahead and use BlobTypeDescriptor.STREAM_BINDING instead of DEFAULT [04/01/11 16:09] --> namemon has joined this channel (~namemon(a)2.49.46.16). [04/01/11 16:09] <sebersole> but is basing that on which *instance* you have really right? [04/01/11 16:09] <gbadner> maybe its 6 of 1 ..., extending the BlobType or extending the dialect [04/01/11 16:11] <sebersole> and again, they alsways have the option to extend BlobType and override nullSafeSet [04/01/11 16:12] <sebersole> which is presumably why they are extending it in the first place [04/01/11 16:12] <gbadner> yes, that's true; but we already have a SQL type descriptor that will work for them [04/01/11 16:12] <sebersole> afaik you made it the type that does this remapping? [04/01/11 16:13] <sebersole> right you did [04/01/11 16:14] <sebersole> nullSafeSet and nulSafeSet do the remapping [04/01/11 16:14] <gbadner> if AbstractStandardBasic.getSqlTypeDescriptor() returns a "standard" SQL type descriptor, then Hibernate can override [04/01/11 16:14] <sebersole> so my overrinden form simply uses BlobTypeDescriptor.STREAM_BINDING [04/01/11 16:15] <gbadner> right; hibernate will just use it [04/01/11 16:15] <sebersole> huh? [04/01/11 16:15] <sebersole> *you* use it [04/01/11 16:15] <gbadner> yes [04/01/11 16:15] <sebersole> *you* are writing the type [04/01/11 16:15] <sebersole> *our* type does remapping [04/01/11 16:16] <gbadner> right [04/01/11 16:16] <sebersole> also, what was the reason for routing this through typeresolver? [04/01/11 16:17] <-- adamw1pl has left this server (Quit: adamw1pl). [04/01/11 16:17] <gbadner> because it deals with a type used for a particular dialect [04/01/11 16:17] <sebersole> what i mean is why not just ask the dialect [04/01/11 16:17] <gbadner> it made sense to me to get type stuff via the TypeResolver [04/01/11 16:18] <sebersole> k [04/01/11 16:18] <gbadner> the TypeResolver can get it from the dialect, or maybe in the future it can get it from somewhere else [04/01/11 16:18] <gbadner> I'd like to minimize the WrapperOptions API {noformat} -- 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