hibernate-orm icon indicating copy to clipboard operation
hibernate-orm copied to clipboard

proposal for @DefaultSchema

Open gavinking opened this issue 2 years ago • 13 comments

This PR introduces an @DefaultSchema annotation, allowing setting the schema at the package or outer class level.

Thoughts?

gavinking avatar Jan 17 '22 15:01 gavinking

Dunno, I always thought that hard-coding a schema name is bad practice and one should rather use a variable that is substituted.

beikov avatar Jan 17 '22 15:01 beikov

@beikov it sounds like you're thinking about a different problem, though I can only speculate what it might be.

The issue this proposal seeks to solve is that there are currently only two levels of granularity for specifying a schema:

  • global to the SessionFactory, or
  • in a particular @Table annotation.

The global setting strikes me as pretty useless, since you can usually handle that with a JDBC parameter. (But perhaps that's not true for all databases?)

The table-level granularity is almost never what I want.

Instead, the sort of thing I often have is, say, 5 entities as inner classes of a test class, and sometimes I get table name collisions with inner classes used by other tests. So it would be nice to be able to easily give each test class its own schema.

Similarly, I would sometimes like to be able to assign all entities in a package to one schema.

I don't see how this as "hardcoding" any more than specifying a table or column name is "hardcoding". It's using a schema to disambiguate name collisions, just like we use packages for this in Java.

gavinking avatar Jan 17 '22 16:01 gavinking

Well, I was treating a schema more like a "tenant namespace" in the past, but I understand that it can also be used for grouping of logically connected tables, so that's fine.

beikov avatar Jan 17 '22 17:01 beikov

Yep, using schemas for tenant segregation is all good, but a completely different usecase.

gavinking avatar Jan 17 '22 17:01 gavinking

Note that the reason that the tests are failing here is that this PR depends on a change to commons-annotations: https://github.com/hibernate/hibernate-commons-annotations/pull/18

gavinking avatar Jan 17 '22 17:01 gavinking

Instead, the sort of thing I often have is, say, 5 entities as inner classes of a test class, and sometimes I get table name collisions with inner classes used by other tests. So it would be nice to be able to easily give each test class its own schema.

Similarly, I would sometimes like to be able to assign all entities in a package to one schema.

These points strike a chord with me, definitely +1 for such benefits with testing

Sanne avatar Jan 18 '22 11:01 Sanne

BTW, somehow related: we occasionally talk about the need to eventually be able to run the testsuite in parallel. Today we run paralle jobs for different database dialects, but it might be useful to run also tests from the same DB Dialect in sub-groups.

Having a declarative way expressing which schema a certain test will use should be useful to get into that sort of direction.

Sanne avatar Jan 18 '22 11:01 Sanne

Having a declarative way expressing which schema a certain test will use should be useful to get into that sort of direction.

I think we want to be careful here. Part of the reason (a large part) the tests take so long is because they are constantly creating and dropping schemas. Simply segmenting packages or other groups of tests to create and drop even more schemas is only going to make that aspect worse.

Would the supposed performance benefit of being able to run the tests in parallel outweigh this aspect? Not sure.

sebersole avatar Jan 18 '22 13:01 sebersole

Not to mention, for tests I think it might be better to have this automated in some fashion rather than relying on specific annotations like this. Think about something like the standard domain models we use in the test suite. Those won't be able to effectively use these dialect override annotations. To be able to properly parallelize tests that use these standard domain models we'd really need some kind of injected way to specify the schema and/or catalog rather than relying on the domain model to include the alternate schemas.

This discussion is kind of beyond the topic of this pull request, so I'll keep it brief. My intention with those standard domain models was always to pre-export them for the entire test suite rather than recreating them for every test that uses them. Which I think will help the performance of the test suite overall. I think the tests that use those standard domain models should just be forced to run in sequence. Others that use one-off models can certainly benefit from segmenting them into different schemas and/or catalogs

sebersole avatar Jan 18 '22 13:01 sebersole

FTR, I was more thinking of Hibernate Reactive's test suite, which is not quite as "extreme" as core.

gavinking avatar Jan 18 '22 13:01 gavinking

Hi Steve, yes good points.

A unified schema would certainly be effective, but to get there it might still be useful to be able to logically group those tests belonging to such schema - ofc this could be done in a number of ways, but this one feature could be useful to get there.

Also I'm confident that while many tests could use such an approach, we'll always have also a significant number of tests which will need alternative strategies - schema isolation would help for some (and yet not all) of these.

Sanne avatar Jan 18 '22 14:01 Sanne

ah, only saw your second comment after posting - sorry that might be confusing.

I agree with your ideas, that might be even better. Still, this shouldn't hurt to get there.

Sanne avatar Jan 18 '22 14:01 Sanne

So, there's a problem with this: there's no good way for the previous-generation id generators to get access to the @DefaultSchema of the entities they belong to.

There might be in the new infrastructure that's currently incubating, but that's not been implemented yet.

So I guess I need to put this on ice for now.

gavinking avatar Jan 21 '22 13:01 gavinking