Skip to content

@Table Annotation is not honoured when joining on tables other than the aggregate root #1147

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
janmax opened this issue Jan 28, 2022 · 3 comments
Assignees
Labels
in: mapping Mapping and conversion infrastructure type: documentation A documentation update

Comments

@janmax
Copy link
Contributor

janmax commented Jan 28, 2022

Hi, I ran into a problem with a CrudRepository and aggregates that contain one-to-many relations and were annotated with @Table. Consider this example:

public interface AuthorRepository extends CrudRepository<AuthorRepository.AuthorRecord, String> {

  @Table("author")
  record AuthorRecord(@Id UUID id, String name, Set<Book> books) {}

  record Book(String title) {}
}

with the following schema:

CREATE TABLE author
(
    id   uuid PRIMARY KEY DEFAULT gen_random_uuid(),
    name text
);

CREATE TABLE book
(
    author uuid,
    title  text
);

Now if I execute the following test (BaseTestConfiguration just initialises a connection to a Postgres database).

class AuthorRepositoryIntegrationTest extends BaseTestConfiguration {

  @Autowired AuthorRepository authorRepository;

  @Test
  void should_persist_and_load_authors() {
    // arrange
    var authorRecord =
        new AuthorRepository.AuthorRecord(
            null, "authorName", Set.of(new AuthorRepository.Book("bookTitle")));

    // act
    authorRepository.save(authorRecord);
  }
}

it throws this error:

Caused by: org.springframework.jdbc.BadSqlGrammarException: PreparedStatementCallback; bad SQL grammar [INSERT INTO "book" ("author_record", "title") VALUES (?, ?)]; nested exception is org.postgresql.util.PSQLException: ERROR: column "author_record" of relation "book" does not exist

To me it looks like the @Table annotation is not honoured. Once I rename AuthorRecord to Author and drop the Annotation everything works fine. (Switching records to classes doesn't change the result.)

During debugging I stumbled upon org.springframework.data.relational.core.mapping.NamingStrategy#getTableName, were the table name is derived from the type. I assume the problem could be fixed by looking for the annotation before defaulting to type.getSimpleName(). I'm happy to try to submit a fix for it if necessary.

pom.xml
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
	xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
	<modelVersion>4.0.0</modelVersion>
	<parent>
		<groupId>org.springframework.boot</groupId>
		<artifactId>spring-boot-starter-parent</artifactId>
		<version>2.6.3</version>
		<relativePath/> <!-- lookup parent from repository -->
	</parent>
	<groupId>com.example</groupId>
	<artifactId>demo</artifactId>
	<version>0.0.1-SNAPSHOT</version>
	<name>demo</name>
	<description>Demo project for Spring Boot</description>
	<properties>
		<java.version>17</java.version>
		<testcontainers.version>1.16.2</testcontainers.version>
	</properties>
	<dependencies>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-data-jdbc</artifactId>
		</dependency>
		<dependency>
			<groupId>org.flywaydb</groupId>
			<artifactId>flyway-core</artifactId>
		</dependency>

		<dependency>
			<groupId>org.postgresql</groupId>
			<artifactId>postgresql</artifactId>
			<scope>runtime</scope>
		</dependency>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-test</artifactId>
			<scope>test</scope>
		</dependency>
		<dependency>
			<groupId>org.testcontainers</groupId>
			<artifactId>junit-jupiter</artifactId>
			<scope>test</scope>
		</dependency>
		<dependency>
			<groupId>org.testcontainers</groupId>
			<artifactId>postgresql</artifactId>
			<scope>test</scope>
		</dependency>
	</dependencies>
	<dependencyManagement>
		<dependencies>
			<dependency>
				<groupId>org.testcontainers</groupId>
				<artifactId>testcontainers-bom</artifactId>
				<version>${testcontainers.version}</version>
				<type>pom</type>
				<scope>import</scope>
			</dependency>
		</dependencies>
	</dependencyManagement>

	<build>
		<plugins>
			<plugin>
				<groupId>org.springframework.boot</groupId>
				<artifactId>spring-boot-maven-plugin</artifactId>
			</plugin>
		</plugins>
	</build>

</project>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 28, 2022
@schauder
Copy link
Contributor

This behaviour is actually intended this way, and probably quite some users probably depend on it by now.
Therefore the fix should probably be to document it accordingly.

@schauder
Copy link
Contributor

schauder commented Feb 8, 2022

We decided to split this issue into three.

The current one should be just the documentation update.

#1161 makes the behavior configurable.
#1162 switches the default with the 3.0 release.

@janmax
Copy link
Contributor Author

janmax commented Jul 12, 2022

I'm happy to look into this (and #1161 and #1162) and will try to come up with a PR as soon as I find the time (hopefully this week).

schauder added a commit that referenced this issue Sep 7, 2022
The default version is the behaviour that existed so far:
The back reference is the table name as generated by the `NamingStrategy` without taking `@Table` annotations into account.

The new alternative is to take `@Table` into account.

The behaviour can be configured by setting the `foreignKeyNaming` property on the `RelationalMappingContext`.

Closes #1161
Closes #1147
schauder added a commit that referenced this issue Sep 7, 2022
The new default is to take `@Table` annotations into account.

The behaviour can be configured by setting the `foreignKeyNaming` property on the `RelationalMappingContext`.

Closes #1162
See #1147
mp911de pushed a commit that referenced this issue Oct 5, 2022
The default version is the behavior that existed so far:
The back reference is the table name as generated by the `NamingStrategy` without taking `@Table` annotations into account.

The new alternative is to take `@Table` into account.

The behavior can be configured by setting the `foreignKeyNaming` property on the `RelationalMappingContext`.

Closes #1161
Closes #1147
mp911de pushed a commit that referenced this issue Oct 5, 2022
The new default is to take `@Table` annotations into account.

The behaviour can be configured by setting the `foreignKeyNaming` property on the `RelationalMappingContext`.

Closes #1162
See #1147
@mp911de mp911de added this to the 3.0 RC1 (2022.0.0) milestone Oct 7, 2022
@mp911de mp911de closed this as completed in 40446f9 Oct 7, 2022
mp911de pushed a commit that referenced this issue Oct 7, 2022
The new default is to take `@Table` annotations into account.

The behaviour can be configured by setting the `foreignKeyNaming` property on the `RelationalMappingContext`.

Closes #1162
See #1147
Original pull request: #1324.
mp911de added a commit that referenced this issue Oct 7, 2022
Do not expose setForeignKeyNaming methods on NamingStrategy to make less assumptions about how a naming strategy gets implemented. Provide getRequiredLeafEntity method on PersistentPropertyPathExtension to reduce null and state assertion checks.

Refine getTableName/getQualifiedTableName approach to reduce API surface and avoid deprecations.

See #1147
Original pull request: #1324.
@schauder schauder added type: documentation A documentation update and removed in: documentation labels Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: mapping Mapping and conversion infrastructure type: documentation A documentation update
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants