Skip to content

Caching of Property Dialect in class Configuration.Mapping #1950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ValResnick opened this issue Dec 28, 2018 · 7 comments · Fixed by #1951
Closed

Caching of Property Dialect in class Configuration.Mapping #1950

ValResnick opened this issue Dec 28, 2018 · 7 comments · Fixed by #1951

Comments

@ValResnick
Copy link

This Property should be cached, because the creation of Dialect can be expensive.

This is our scenario: We have a base class with a lot of subclasses (over 400).

The class UnionSubclassEntityPersister creates for our base class a very long (over 20 Million characters) SubQuery in GenerateSubquery.

// from UnionSubclassEntityPersister
subquery = GenerateSubquery(persistentClass, mapping);

Everytime GetSqlTypeCode is called for an instance of NullableType the Dialect will be created again.

// from UnionSubclassEntityPersister.GenerateSubquery
var sqlType = col.GetSqlTypeCode(mapping);
// from NullableType
static SqlType OverrideSqlType(IMapping mapping, SqlType type)
{
   return mapping != null ? mapping.Dialect.OverrideSqlType(type) : type;
}

This is a possible solution for the problem.

[Serializable]
private class Mapping : IMapping
{
   private readonly Configuration configuration;

   [NonSerialized]
   private readonly Lazy<Dialect.Dialect> _lazyDialect;

   public Mapping(Configuration configuration)
   {
      this.configuration = configuration;
				
      _lazyDialect = new Lazy<Dialect.Dialect>(() => NHibernate.Dialect.Dialect.GetDialect(this.configuration.Properties));
   }
   
   ...

   public Dialect.Dialect Dialect => _lazyDialect.Value;
}
@fredericDelaporte
Copy link
Member

Have you checked there is still the issue with NHibernate v5.2? There have been some changes done on that precise subject. See #1703.

@ValResnick
Copy link
Author

Today i tried to upgrade our project to V5.2.1. That's the reason i have created this issue :-)

@fredericDelaporte
Copy link
Member

Ok, it is already lazy in Mappings, but the trouble is in Mapping.

This dates back to 1464f48, #703, v5.0.

@fredericDelaporte
Copy link
Member

In this case, switching to lazy will cause bugs if someone build a session factory from a configuration, then change the configuration's dialect (and connection string, obviously) and build a new session factory from it. (Or use other methods depending on that Mapping class.) Something else would have to be done.

@ValResnick
Copy link
Author

My solution is not (fully) correct. When the mapping was deserialized, the Property throws a Nullpointer Exception.

@ValResnick
Copy link
Author

ValResnick commented Dec 28, 2018

Can i avoid the creation of the big subQuery instead?
I assume a query with 20 Million characters makes no sense for the framework ?

@fredericDelaporte
Copy link
Member

Can i avoid the creation the big subQuery instead?

This is another subject. I think the dialect many instantiations in Mapping should anyway be fixed, so let the issue here stay on this dialect subject.

About the big sub-query trouble, it is likely a subject to be discussed first on the user groups (nhusers). It seems it is useless in your case (and likely not working if trying to execute it), so it should be checked if it can be avoided. That is a support subject, whereas the NHibernate GitHub issues are used for bug reports or feature requests. (It may conclude there would be a need for changes in NHibernate, in which case another issue will have to be opened, but it has first to be checked.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants