feat: support named schemas #1946
Conversation
rayudu3745
force-pushed
the
named_schemas
branch
3 times, most recently
from
fe857d7 to
d3ef358
Compare
rayudu3745
force-pushed
the
named_schemas
branch
from
d3ef358 to
84ff8d4
Compare
|
|
||
| String defaultSchema = | ||
| (String) options.getConfigurationValues().get(AvailableSettings.DEFAULT_SCHEMA); | ||
|
|
There was a problem hiding this comment.
nit (here and elsewhere): I don't think we need to introduce empty lines before every occurrence of defaultSchema. Would you mind taking a second look where it makes sense to break up the code a bit, and where it is not needed (e.g. I think that having this line of code separated by an empty line both before and after is a bit too much)
| "drop table `Employee`", | ||
| "drop table `Employee_Sequence`", | ||
| "drop table Employee", | ||
| "drop table Employee_Sequence", |
There was a problem hiding this comment.
Why does this change remove identifier quoting? I think that this might break existing customers who use table names that contain reserved keywords.
There was a problem hiding this comment.
changed how the name is rendered correctly with sqlContext.format method ,
quoting will still be applied by hibernate to identifier when required like if sqlkeyword is used, here it is not required
there is fix regarding this which i added to SpannerDialect in hibernate repository recently but that is not released yet, so added that fix here as well now
also added a test to verify the quotes are applied when a key word is applied
...dialect/src/test/java/com/google/cloud/spanner/hibernate/SchemaGenerationMockServerTest.java
Outdated
Show resolved
Hide resolved
| IntStream.range(0, fetchSize) | ||
| .mapToObj( | ||
| ignore -> "nextval('" + sequenceName.getSequenceName().getText() + "') as n") | ||
| .mapToObj(ignore -> "nextval('" + sequenceName.render() + "') as n") |
There was a problem hiding this comment.
Do you have any tests that verify that this works? If I understand it correctly, it will generate a statement like select next('my_schema.my_sequence') as n. Is that correct?
There was a problem hiding this comment.
yes , correct
there is a test for this in NamedSchemaIT test
| The Cloud Spanner Dialect supports named schemas. You can organize your tables into specific Spanner schemas using the standard JPA/Hibernate mechanisms. For more information on Spanner Named Schemas, please refer to the https://docs.cloud.google.com/spanner/docs/named-schemas[official documentation]. | ||
|
|
||
| You can specify a schema for a specific entity using the `schema` attribute of the `@Table` annotation: | ||
|
|
There was a problem hiding this comment.
I think that we should add a note here that named schemas cannot (currently) be used to implement a multi-tenancy strategy, as Spanner does not support a 'search_path' or similar property.
More background information:
See this blog for an example for how that can be used with PostgreSQL: https://vladmihalcea.com/hibernate-database-schema-multitenancy/
There was a problem hiding this comment.
Done, added a not saying schema based multi-tenancy is not supported
we might have to support sql statements like 'set schema' in driver, will look into it more if customer asks for it
2 participants
Add support for spanner named schemas https://docs.cloud.google.com/spanner/docs/named-schemas