Skip to content

HHH-19559 add hibernate.multi_tenant.set_schema #10372

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.

Sign up for GitHub

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
wants to merge 8 commits into from
Closed

HHH-19559 add hibernate.multi_tenant.set_schema #10372

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
67363e9
HHH-19559 add hibernate.multi_tenant.set_schema
gavinking Jun 19, 2025
c99e2ee
HHH-19559 some related refactoring
gavinking Jun 27, 2025
af6bfbf
HHH-19559 move management of the schema into LogicalConnectionManaged…
gavinking Jun 27, 2025
ac01280
HHH-19559 introduce TenantSchemaMapper
gavinking Jun 27, 2025
949873b
HHH-19559 add appropriate @Incubating annotations
gavinking Jun 27, 2025
8b8d4bb
squash! HHH-19559 move management of the schema into LogicalConnectio…
gavinking Jun 27, 2025
755ea99
add ability to target a given schema/catalog in SchemaManager
gavinking Jun 27, 2025
19828e0
HHH-19559 test for schema-based multitenancy
gavinking Jun 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ public class SessionFactoryOptionsBuilder implements SessionFactoryOptions {
// multi-tenancy
private boolean multiTenancyEnabled;
private CurrentTenantIdentifierResolver<Object> currentTenantIdentifierResolver;
private boolean setTenantSchemaEnabled;

// Queries
private SqmFunctionRegistry sqmFunctionRegistry;
Expand Down Expand Up @@ -371,6 +372,7 @@ public SessionFactoryOptionsBuilder(StandardServiceRegistry serviceRegistry, Boo
null
);
}
setTenantSchemaEnabled = configurationService.getSetting( SET_TENANT_SCHEMA, BOOLEAN, false );

delayBatchFetchLoaderCreations =
configurationService.getSetting( DELAY_ENTITY_LOADER_CREATIONS, BOOLEAN, true );
Expand Down Expand Up @@ -1003,6 +1005,11 @@ public boolean isMultiTenancyEnabled() {
return multiTenancyEnabled;
}

@Override
public boolean isSetTenantSchemaEnabled() {
return setTenantSchemaEnabled;
}

@Override
public CurrentTenantIdentifierResolver<Object> getCurrentTenantIdentifierResolver() {
return currentTenantIdentifierResolver;
Expand Down Expand Up @@ -1450,6 +1457,10 @@ public void applyCurrentTenantIdentifierResolver(CurrentTenantIdentifierResolver
this.currentTenantIdentifierResolver = (CurrentTenantIdentifierResolver<Object>) resolver;
}

public void applySetTenantSchema(boolean enabled) {
this.setTenantSchemaEnabled = enabled;
}

public void enableNamedQueryCheckingOnStartup(boolean enabled) {
this.namedQueryStartupCheckingEnabled = enabled;
}
Expand Down
5 changes: 5 additions & 0 deletions ...te-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,11 @@ public boolean isMultiTenancyEnabled() {
return delegate.isMultiTenancyEnabled();
}

@Override
public boolean isSetTenantSchemaEnabled() {
return delegate.isSetTenantSchemaEnabled();
}

@Override
public CurrentTenantIdentifierResolver<Object> getCurrentTenantIdentifierResolver() {
return delegate.getCurrentTenantIdentifierResolver();
Expand Down
11 changes: 11 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,17 @@ default SessionEventListener[] buildSessionEventListeners() {
*/
boolean isMultiTenancyEnabled();

/**
* Should the schema be {@linkplain java.sql.Connection#setSchema set} to the
* {@linkplain CurrentTenantIdentifierResolver#schemaName schema belonging to
* the current tenant} each time a connection is obtained?
*
* @see org.hibernate.cfg.MultiTenancySettings#SET_TENANT_SCHEMA
*
* @since 7.1
*/
boolean isSetTenantSchemaEnabled();

/**
* Obtain a reference to the
* {@linkplain CurrentTenantIdentifierResolver current tenant identifier resolver},
Expand Down
15 changes: 14 additions & 1 deletion hibernate-core/src/main/java/org/hibernate/cfg/MultiTenancySettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public interface MultiTenancySettings {
* either:
* <ul>
* <li>an instance of {@code CurrentTenantIdentifierResolver},
* <li>a {@link Class} representing an class that implements {@code CurrentTenantIdentifierResolver}, or
* <li>a {@link Class} representing a class that implements {@code CurrentTenantIdentifierResolver}, or
* <li>the name of a class that implements {@code CurrentTenantIdentifierResolver}.
* </ul>
*
Expand All @@ -42,4 +42,17 @@ public interface MultiTenancySettings {
* This setting configures the name of the DataSource to use for this access
*/
String TENANT_IDENTIFIER_TO_USE_FOR_ANY_KEY = "hibernate.multi_tenant.datasource.identifier_for_any";

/**
* Specifies that {@link java.sql.Connection#setSchema(String)}}
* should be called with the current tenant id, or with the schema name
* returned by {@link CurrentTenantIdentifierResolver#schemaName}.
*
* @settingDefault false
*
* @see CurrentTenantIdentifierResolver#schemaName
*
* @since 7.1
*/
String SET_TENANT_SCHEMA = "hibernate.multi_tenant.set_schema";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love the naming here at all; unfortunately the existing properties have irregular naming.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that was an unfortunate reality of a singular AvailableSettings with a zagillabillion settings. Related things weren't always "geographically" close to one another and naming tended to get disjointed. It was the main reason I started breaking them out.

But we can always "shadow deprecate" them - aka create new ones that replace the old while still recognizing the old. See DeprecationLogger#deprecatedSetting(java.lang.String, java.lang.String)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we can always "shadow deprecate" them - aka create new ones that replace the old while still recognizing the old.

Yeah, that would be reasonable, I think.

}
4 changes: 2 additions & 2 deletions hibernate-core/src/main/java/org/hibernate/context/spi/AbstractCurrentSessionContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ public SessionFactoryImplementor factory() {

protected SessionBuilder baseSessionBuilder() {
final SessionBuilderImplementor builder = factory.withOptions();
final CurrentTenantIdentifierResolver<Object> resolver = factory.getCurrentTenantIdentifierResolver();
final var resolver = factory.getCurrentTenantIdentifierResolver();
if ( resolver != null ) {
builder.tenantIdentifier( resolver.resolveCurrentTenantIdentifier() );
}
return builder;
}

protected void validateExistingSession(Session existingSession) {
final CurrentTenantIdentifierResolver<Object> resolver = factory.getCurrentTenantIdentifierResolver();
final var resolver = factory.getCurrentTenantIdentifierResolver();
if ( resolver != null && resolver.validateExistingCurrentSessions() ) {
final Object currentValue = resolver.resolveCurrentTenantIdentifier();
final JavaType<Object> tenantIdentifierJavaType = factory.getTenantIdentifierJavaType();
Expand Down
18 changes: 18 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/context/spi/CurrentTenantIdentifierResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/
package org.hibernate.context.spi;

import org.checkerframework.checker.nullness.qual.NonNull;

/**
* A callback registered with the {@link org.hibernate.SessionFactory} that is
* responsible for resolving the current tenant identifier.
Expand Down Expand Up @@ -48,4 +50,20 @@ public interface CurrentTenantIdentifierResolver<T> {
default boolean isRoot(T tenantId) {
return false;
}

/**
* The name of the database schema for data belonging to the tenant with the
* given identifier.
* <p>
* Called when {@value org.hibernate.cfg.MultiTenancySettings#SET_TENANT_SCHEMA}
* is enabled.
*
* @param tenantIdentifier The tenant identifier
* @return The name of the database schema belonging to that tenant
*
* @see org.hibernate.cfg.MultiTenancySettings#SET_TENANT_SCHEMA
*/
default @NonNull String schemaName(@NonNull T tenantIdentifier) {
return tenantIdentifier.toString();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels... out of place? I'm always a bit weirded out by "optionally relevant" methods like this.

Beyond feelings, when I think about it... My main argument in this case would be that the tenant identifier can be provided explicitly when opening a session, so I'm not sure a CurrentTenantIdentifierResolver would necessarily always make sense in an application. In which case you'd have no way of leveraging this feature, except perhaps implementing all the other methods by throwing exceptions.

Can't we instead have an interface to turn an identifier name to an schema name, and ask people to have hibernate.multi_tenant.schema_resolver (or whatever) to an implementation? We can totally have a simple implementation that just takes the tenant identifier as the schema name.

That way, I think the "schema setting" part could be implemented similarly to discriminator-based multitenancy. Connections would be irrelevant.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way... I get the feeling this could be a much larger concept. Like, an interface that resolves a tenant ID into a "datasource ID" (passed to connection resolvers) and schema name and discriminator (for discriminator-based multitenancy, so that discriminators in the DB aren't necessarily exactly the tenant ID).
Not suggesting to do this here, but it does feel... clean? Maybe I'm just being weird.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels... out of place? I'm always a bit weirded out by "optionally relevant" methods like this.

Well that's what I meant when I said it's not completely conceptually clean.

But it's sure as hell convenient for the user. If I'm a user of this stuff I would much prefer to have all multitenancy-related shit located in just one class.

Can't we instead have an interface to turn an identifier name to an schema name, and ask people to have hibernate.multi_tenant.schema_resolver (or whatever) to an implementation? We can totally have a simple implementation that just takes the tenant identifier as the schema name.

I mean, sure, of course we can.

But it's just such a god damn clusterfuck as far as usability goes. How many properties are you going to have to set, and how many interfaces are you having to implement, to get this one trivial feature working?

Like, an interface that resolves a tenant ID into a "datasource ID" (passed to connection resolvers) and schema name and discriminator (for discriminator-based multitenancy, so that discriminators in the DB aren't necessarily exactly the tenant ID).

I guess I think this would be jumping the shark.

Copy link
Member

@yrodiere yrodiere Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like, an interface that resolves a tenant ID into a "datasource ID" (passed to connection resolvers) and schema name and discriminator (for discriminator-based multitenancy, so that discriminators in the DB aren't necessarily exactly the tenant ID).

I guess I think this would be jumping the shark.

It's not that different from naming strategies, but sure, it's complex.

Anyway, my point was: you want to merge two responsibilities in the same interface to minimize complexity, but:

  1. These particular two responsibilities can be used independently (one without the other), so this creates complexity for those needing only one. Which I guess is fine if we consider people generally need both.
  2. The derivation of a schema name from a tenant ID is itself a responsibility that could possibly have close siblings, such as deriving a discriminator from a tenant ID.

I would add that finding the current tenant ID generally has to do with the higher layers of your application (e.g. inspecting user connection info), while turning a tenant ID to a schema name is clearly about the lower layers (DBA stuff). So it's really a bit weird to mix those.

If I had to start from scratch with a single entry point for the whole thing, with some cohesion, I'd expose something like this:

interface MultiTenancyStrategy {

    // Optionals could just be nullable. Whatever.

    Optional<MultiTenantConnectionProvider> createConnectionProvider( .... );

    Optional<String> tenantIdToSchemaName(String tenantId);

    // Could be a type parameter instead of Object, but IMO if we don't check consistency on startup, that's hiding problems under the carpet.

    Object tenantIdToDiscriminator(String tenantId);

    String discriminatorToTenantId(Object tenantId);

}

EDIT: With default methods, obviously.

Then you can have a default "discriminator" strategy that just takes the tenant ID as a discriminator, an opt-in (but built-in) "schema" strategy that takes the tenant ID as schema name, and custom strategies from everything else. And the entry point is clear and exposes everything you need to know.

That being said... I'm certainly not blocking you from doing that. I just disagree.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yrodiere Note that I'm not against having all these three things as (almost) completely independent features, in the sense that the user is allowed to turn each on and off independently. That's more-or-less what I've implemented. Though it's not completely realized because I didn't add support for setSchema() to ContextualJdbcConnectionAccess. Or rather: I added it then removed it.

But Steve on the other hand is pushing more in the direction of making these things mutually-exclusive, more like the model which existed implicitly on the old enum.

I'm fine either way; I just wouldn't add "can use them all together" as a requirement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yoann disagrees with me?!? No way ;)

The mapping of identifier in the app to tenant-name in the database can actually be a shared contract. We could even apply it in the @TenantId case. And can be as simple as a conversion:

interface TenantNameMapper {
    /**
     * Maps the name of the tenant used in the application (aka, CurrentTenantIdentifierResolver#resolveCurrentTenantIdentifier) to the corresponding database name which might be a schema name or 
    String toDatabaseName(Object identifier);
    Object toIdentifier(String databaseName);
}

TenantNameMapper#toDatabaseName allows taking, e.g., CurrentTenantIdentifierResolver#resolveCurrentTenantIdentifier and mapping it to a value that depends a bit on the overall strategy:

  1. For column-based, this would be the value stored in the column
  2. For MultiTenantConnectionProvider, this would become the valiue passed to its #getConnection method
  3. Here it would be the name of the schema we switch to.

#toIdentifier is obviously the inverse of this. And maybe not even needed depending

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I don't think I am "pushing more in the direction of making these things mutually-exclusive" so much as trying to recognize the bits that are reusable across the different situations and designing the SPI around that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yoann disagrees with me?!? No way ;)

What can I say, I'm a sucker for traditions! And we both like this one ;)

interface TenantNameMapper {

This makes a lot of sense to me, and is more or less in line with my initial proposal:

Can't we instead have an interface to turn an identifier name to an schema name, and ask people to have hibernate.multi_tenant.schema_resolver (or whatever) to an implementation? We can totally have a simple implementation that just takes the tenant identifier as the schema name.

... but (to keep the tradition alive):

  1. toIdentifier is pointless in the schema case (I think?)

  2. Gavin doesn't like this. Quoting:

    I mean, sure, of course we can.

    But it's just such a god damn clusterfuck as far as usability goes. How many properties are you going to have to set, and how many interfaces are you having to implement, to get this one trivial feature working?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toIdentifier is pointless in the schema case (I think?)

Yep, as I said:

#toIdentifier is obviously the inverse of this. And maybe not even needed depending

However, (1) it does not hurt and (2) I'd much prefer a single way to do that across all "strategies".

Gavin doesn't like this. Quoting:

In the normal case you don't even implement/provide this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yrodiere I have introduced TenantSchemaMapper and replaced the previously-proposed property setting with hibernate.multi_tenant.schema_mapper. Supplying a TenantSchemaMapper turns on schema-based multitenancy.

}
12 changes: 12 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@
import org.hibernate.stat.SessionStatistics;
import org.hibernate.type.format.FormatMapper;

import java.sql.Connection;
import java.sql.SQLException;
import java.util.Collection;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -1241,4 +1243,14 @@ public Object loadFromSecondLevelCache(EntityPersister persister, EntityKey enti
public boolean isIdentifierRollbackEnabled() {
return delegate.isIdentifierRollbackEnabled();
}

@Override
public void afterObtainConnection(Connection connection) throws SQLException {
delegate.afterObtainConnection( connection );
}

@Override
public void beforeReleaseConnection(Connection connection) throws SQLException {
delegate.beforeReleaseConnection( connection );
}
}
29 changes: 29 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionContractImplementor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/
package org.hibernate.engine.spi;

import java.sql.Connection;
import java.sql.SQLException;
import java.util.Set;
import java.util.UUID;
import jakarta.persistence.TransactionRequiredException;
Expand Down Expand Up @@ -572,4 +574,31 @@ default boolean isStatelessSession() {
*/
@Incubating
Object loadFromSecondLevelCache(EntityPersister persister, EntityKey entityKey, Object instanceToLoad, LockMode lockMode);

/**
* Called after a non-contextual JDBC connection is obtained.
* <p>
* Sets the schema to the schema belonging to the current tenant if:
* <ol>
* <li>{@value org.hibernate.cfg.MultiTenancySettings#SET_TENANT_SCHEMA} is enabled, and
* <li>this session has an active tenant id.
* </ol>
*
* @param connection A JDBC connection which was just acquired
*
* @since 7.1
*/
void afterObtainConnection(Connection connection) throws SQLException;

/**
* Called right before non-contextual a JDBC connection is released.
* <p>
* Unset the schema which was set by {@link #afterObtainConnection},
* if any.
* .
* @param connection The JDBC connection which is about to be released
*
* @since 7.1
*/
void beforeReleaseConnection(Connection connection) throws SQLException;
}
12 changes: 12 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionDelegatorBaseImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import org.hibernate.resource.transaction.spi.TransactionCoordinator;
import org.hibernate.type.format.FormatMapper;

import java.sql.Connection;
import java.sql.SQLException;
import java.util.List;
import java.util.Set;
import java.util.TimeZone;
Expand Down Expand Up @@ -697,4 +699,14 @@ public Object loadFromSecondLevelCache(EntityPersister persister, EntityKey enti
public boolean isIdentifierRollbackEnabled() {
return delegate.isIdentifierRollbackEnabled();
}

@Override
public void afterObtainConnection(Connection connection) throws SQLException {
delegate.afterObtainConnection( connection );
}

@Override
public void beforeReleaseConnection(Connection connection) throws SQLException {
delegate.beforeReleaseConnection( connection );
}
}
4 changes: 1 addition & 3 deletions hibernate-core/src/main/java/org/hibernate/generator/internal/TenantIdGeneration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import org.hibernate.PropertyValueException;
import org.hibernate.annotations.TenantId;
import org.hibernate.context.spi.CurrentTenantIdentifierResolver;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.generator.BeforeExecutionGenerator;
Expand Down Expand Up @@ -51,8 +50,7 @@ public Object generate(SharedSessionContractImplementor session, Object owner, O
final SessionFactoryImplementor sessionFactory = session.getSessionFactory();
final Object tenantId = session.getTenantIdentifierValue();
if ( currentValue != null ) {
final CurrentTenantIdentifierResolver<Object> resolver =
sessionFactory.getCurrentTenantIdentifierResolver();
final var resolver = sessionFactory.getCurrentTenantIdentifierResolver();
if ( resolver != null && resolver.isRoot( tenantId ) ) {
// the "root" tenant is allowed to set the tenant id explicitly
return currentValue;
Expand Down
34 changes: 31 additions & 3 deletions hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.hibernate.binder.internal.TenantIdBinder;
import org.hibernate.boot.spi.SessionFactoryOptions;
import org.hibernate.cache.spi.CacheTransactionSynchronization;
import org.hibernate.context.spi.CurrentTenantIdentifierResolver;
import org.hibernate.dialect.Dialect;
import org.hibernate.engine.internal.SessionEventListenerManagerImpl;
import org.hibernate.engine.jdbc.LobCreator;
Expand Down Expand Up @@ -107,6 +106,7 @@
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serial;
import java.sql.Connection;
import java.sql.SQLException;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -243,7 +243,7 @@ protected final void setUpMultitenancy(SessionFactoryImplementor factory, LoadQu
throw new HibernateException( "SessionFactory configured for multi-tenancy, but no tenant identifier specified" );
}
else {
final CurrentTenantIdentifierResolver<Object> resolver = factory.getCurrentTenantIdentifierResolver();
final var resolver = factory.getCurrentTenantIdentifierResolver();
if ( resolver==null || !resolver.isRoot( tenantIdentifier ) ) {
// turn on the filter, unless this is the "root" tenant with access to all partitions
loadQueryInfluencers.enableFilter( TenantIdBinder.FILTER_NAME )
Expand Down Expand Up @@ -676,16 +676,18 @@ public boolean isConnected() {

@Override
public JdbcConnectionAccess getJdbcConnectionAccess() {
// See class-level JavaDocs for a discussion of the concurrent-access safety of this method
// See class-level Javadoc for a discussion of the concurrent-access safety of this method
if ( jdbcConnectionAccess == null ) {
if ( !factoryOptions.isMultiTenancyEnabled() ) {
// we might still be using schema-based multitenancy
Comment on lines 681 to +682
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, what?

This might warrant a renaming of isMultiTenancyEnabled()? Perhaps isMultiTenancyEnabledOrNotIDontKnowAskSomeoneElse()?

Copy link
Member Author

@gavinking gavinking Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yoann, that's the whole point of this.

The design that we inherited here was that "Mutitenancy" = you have a MultiTenantConnectionProvider. And that was just wrong, and I guess we need to get away from it.

The problem is that Hibernate has support for all sorts of ConnectionProviders, and you completely lose all of that as soon as you need a MultiTenantConnectionProvider. The only thing we sorta give you is support for the case of a JNDI-bound DataSource.

Copy link
Member Author

@gavinking gavinking Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the old MultiTenancyStrategy enum we used to have, SCHEMA and DATABASE were synonyms and both meant "use a custom MultiTenantConnectionProvider". (And DISCRIMINATOR was simply unimplemented.) Here I'm introducing a totally new thing that we've never had before: a way to do schema-based multitenancy without a MultiTenantConnectionProvider.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know. Sorry, perhaps not the best time for humor.

More seriously, renaming isMultiTenancyEnabled() to isMultiTenantConnectionProviderEnabled would probably be clearer than adding comments like this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More seriously, renaming isMultiTenancyEnabled() to isMultiTenantConnectionProviderEnabled would probably be clearer than adding comments like this?

Sure, but that method's defined by an SPI so it would have to be done via the deprecation process. And note that isMultiTenancyEnabled() is already clearly documented to be about the presence of a MultiTenantConnectionProvider.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The design that we inherited here was that "Mutitenancy" = you have a MultiTenantConnectionProvider. And that was just wrong, and I guess we need to get away from it.

I disagree with this. And you even disagreed with it above.

There are a few situations here -

  1. You have different "Connection pools" based on tenant - this is the purpose of MultiTenantConnectionProvider
  2. You use column-based tenancy
  3. This new hybrid you are implementing which uses plain-old ConnectionProvider and simply switches the schema/catalog of the Connection based on the tenant-id associated with the Session

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sebersole The specific method isMultiTenancyEnabled() always meant that there was a MultiTenantConnectionProvider.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure where I said otherwise, but yep.

jdbcConnectionAccess = new NonContextualJdbcConnectionAccess(
sessionEventsManager,
factory.connectionProvider,
this
);
}
else {
// we're using datasource-based multitenancy
jdbcConnectionAccess = new ContextualJdbcConnectionAccess(
tenantIdentifier,
sessionEventsManager,
Expand All @@ -697,6 +699,32 @@ public JdbcConnectionAccess getJdbcConnectionAccess() {
return jdbcConnectionAccess;
}

private boolean useSchemaBasedMultiTenancy() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we are again :)

return tenantIdentifier != null
&& factory.getSessionFactoryOptions().isSetTenantSchemaEnabled();
}

private String tenantSchema() {
final var tenantIdResolver = factory.getCurrentTenantIdentifierResolver();
return tenantIdResolver == null
? (String) tenantIdentifier
: tenantIdResolver.schemaName( tenantIdentifier );
}

@Override
public void afterObtainConnection(Connection connection) throws SQLException {
if ( useSchemaBasedMultiTenancy() ) {
connection.setSchema( tenantSchema() );
}
}

@Override
public void beforeReleaseConnection(Connection connection) throws SQLException {
if ( useSchemaBasedMultiTenancy() ) {
connection.setSchema( null );
}
}

@Override
public EntityKey generateEntityKey(Object id, EntityPersister persister) {
return new EntityKey( id, persister );
Expand Down
27 changes: 26 additions & 1 deletion hibernate-core/src/main/java/org/hibernate/internal/NonContextualJdbcConnectionAccess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.event.monitor.spi.EventMonitor;
import org.hibernate.event.monitor.spi.DiagnosticEvent;
import org.jboss.logging.Logger;

/**
* @author Steve Ebersole
Expand All @@ -24,6 +25,8 @@ public class NonContextualJdbcConnectionAccess implements JdbcConnectionAccess,
private final ConnectionProvider connectionProvider;
private final SharedSessionContractImplementor session;

private static final Logger log = Logger.getLogger( NonContextualJdbcConnectionAccess.class );

public NonContextualJdbcConnectionAccess(
SessionEventListener listener,
ConnectionProvider connectionProvider,
Expand All @@ -39,9 +42,10 @@ public NonContextualJdbcConnectionAccess(
public Connection obtainConnection() throws SQLException {
final EventMonitor eventMonitor = session.getEventMonitor();
final DiagnosticEvent jdbcConnectionAcquisitionEvent = eventMonitor.beginJdbcConnectionAcquisitionEvent();
final Connection connection;
try {
listener.jdbcConnectionAcquisitionStart();
return connectionProvider.getConnection();
connection = connectionProvider.getConnection();
}
finally {
eventMonitor.completeJdbcConnectionAcquisitionEvent(
Expand All @@ -51,10 +55,31 @@ public Connection obtainConnection() throws SQLException {
);
listener.jdbcConnectionAcquisitionEnd();
}

try {
session.afterObtainConnection( connection );
}
catch (SQLException e) {
try {
releaseConnection( connection );
}
catch (SQLException re) {
e.addSuppressed( re );
}
throw e;
}
return connection;
}

@Override
public void releaseConnection(Connection connection) throws SQLException {
try {
session.beforeReleaseConnection( connection );
}
catch (SQLException e) {
log.warn( "Error before releasing JDBC connection", e );
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sebersole I'm anticipating that you might object to what I'm doing here. If so, I need a better suggestion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the idea is that a user would not need MultiTenantConnectionProvider right? They could just do the normal ConnectionProvider set up and, in conjunction with CurrentTenantIdentifierResolver, we'd just alter the schema/catalog used by the Session - right? Just want to make sure I'm understanding the intent here.

Assuming iiuc couldn't we just imply this? It would be better if there were an actual flag saying that the user wants schema/catalog tenancy. E.g.

hibernate.connection...
hibernate.tenancy.strategy=schema
hibernate.tenant_identifier_resolver=my.ResolverImpl

And that this would just be enough to infer what to do.

Have you looked at incorporating this into the LogicalConnection for the Session? Naively perhaps, that would seem like a better place.

Copy link
Member

@sebersole sebersole Jun 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There used to be a way that users specified the type of tenancy, but I think that got dropped. In a way, this PR adds that distinction back.

Another option would be to infer this when there is a ConnnectionProvider (and NOT a MultiTenantConnectionProvider) with a configured CurrentTenantIdentifierResolver. What I mean is that this may actually be enough:

hibernate.connection...
hibernate.tenant_identifier_resolver=my.ResolverImpl

Trying to get away from the need for a setting here. Or is there some nuance I'm missing that makes you think we need it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the idea is that a user would not need MultiTenantConnectionProvider right?

Yes.

They could just do the normal ConnectionProvider set up and, in conjunction with CurrentTenantIdentifierResolver, we'd just alter the schema/catalog used by the Session - right?

Yes.

It would be better if there were an actual flag saying that the user wants schema/catalog tenancy.

That's ... pretty much what I have implemented here, I believe. I just called the property something different.

Have you looked at incorporating this into the LogicalConnection for the Session? Naively perhaps, that would seem like a better place.

I have not. Will take a look at that.

OTOH, I think I would prefer this not be too deep down in the layers. It's a fairly high-level construct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OTOH, I think I would prefer this not be too deep down in the layers. It's a fairly high-level construct.

I'd argue that this is far deeper than LogicalConnection, which is one of the reasons I mentioned it. Plus LogicalConnection is the "eye of the needle" (in the good way) for Connection-related access for the Session.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But given the 2 explicit options I'd defiitely prefer hibernate.tenancy.style=schema over hibernate.multi_tenant.set_schema=true.

Wouldn't it be a reasonable use case to use both datasource-based and schema-based multitenancy?

E.g. we could have:

tenant1.1 => datasource 1, schema1
tenant1.2 => datasource 1, schema2
tenant2.1 => datasource 2, schema1
tenant2.2 => datasource 2, schema2

Thinking about it, we could even throw discriminator-based multitenancy into the mix, if some tenants share the same datasource + schema...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be a reasonable use case to use both datasource-based and schema-based multitenancy?

Well this is already easily possible by just implementing your own MultiTenantConnectionProvider.

For such complicated scenarios, I think it's totally reasonable that you have to write a bit of code.

The problem I'm trying to solve here is to have a way to do schema-based multitenancy with any regular-ol' ConnectionProvider, without having to provide a MultiTenantConnectionProvider.

Copy link
Member

@yrodiere yrodiere Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point was that if you introduce hibernate.tenancy.style=schema, you'll need to take into account that there's a custom style or whatever you want to call it, that goes beyond just datasource/schema/discriminator. Or that not setting a style is special case. Or that the datasource style actually encompasses all others. Or...

I get you're trying to make simple things simpler, I'd be wary about making complex things more complex or more confusing, because that looks like a very real risk here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point was that if you introduce hibernate.tenancy.style=schema, you'll need to take into account that there's a custom style or whatever you want to call it, that goes beyond just datasource/schema/discriminator

I think this is a good point, and goes to what I suggested with the 3 situations. Phrased in the terms here we have:

  1. column-based (@TenantId)
  2. explicit MultiTenantConnectionProvider-based
  3. ConnectionProvider + "adjustment" of the Connection for the Session

I get you're trying to make simple things simpler, I'd be wary about making complex things more complex or more confusing, because that looks like a very real risk here.

I'm not following this... What is the complex case that you think is getting more complex here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sebersole I have done as you asked, and moved the business end of this into LogicalConnectionManagedImpl.

The cost of it was that now LogicalConnectionManagedImpl has to maintain a reference to the JdbcSessionOwner.


final EventMonitor eventMonitor = session.getEventMonitor();
final DiagnosticEvent jdbcConnectionReleaseEvent = eventMonitor.beginJdbcConnectionReleaseEvent();
try {
Expand Down
Loading
Loading