Skip to content
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

Handles container restarts for already initialized postgresql database #183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Handles container restarts for already initialized postgresql database #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.samples</groupId>
<artifactId>spring-petclinic-rest</artifactId>
<version>3.2.1</version>
<version>3.3.0</version>

<description>REST version of the Spring Petclinic sample application</description>
<url>https://spring-petclinic.github.io/</url>
Expand Down
73 changes: 23 additions & 50 deletions src/main/resources/db/postgres/schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,102 +1,75 @@
-- Create tables only if they don't exist
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: we could maybe refactor and modernize the PostgreSQL script following the work from the canonical version of Spring Petclinic: https://github.com/spring-projects/spring-petclinic/blob/main/src/main/resources/db/postgres/schema.sql and https://github.com/spring-projects/spring-petclinic/blob/main/src/main/resources/db/postgres/data.sql
We should bear in mind that we support 3 DAO implementations

Copy link
Author

Choose a reason for hiding this comment

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

Being closer to the canonical version certainly makes sense to me.

CREATE TABLE IF NOT EXISTS vets (
id SERIAL,
id SERIAL PRIMARY KEY,
Copy link
Member

Choose a reason for hiding this comment

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

question: why did you change to this primary key? I suppose the CONSTRAINT pk_vets PRIMARY KEY (id) was there in order to provide the pk_vets name to the primary key.

Copy link
Author

Choose a reason for hiding this comment

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

You are correct. This should be reverted.

first_name VARCHAR(30),
last_name VARCHAR(30),
CONSTRAINT pk_vets PRIMARY KEY (id)
last_name VARCHAR(30)
);

CREATE INDEX IF NOT EXISTS idx_vets_last_name ON vets (last_name);

ALTER SEQUENCE vets_id_seq RESTART WITH 100;


CREATE TABLE IF NOT EXISTS specialties (
id SERIAL,
name VARCHAR(80),
CONSTRAINT pk_specialties PRIMARY KEY (id)
id SERIAL PRIMARY KEY,
name VARCHAR(80)
);

CREATE INDEX IF NOT EXISTS idx_specialties_name ON specialties (name);

ALTER SEQUENCE specialties_id_seq RESTART WITH 100;


CREATE TABLE IF NOT EXISTS vet_specialties (
vet_id INT NOT NULL,
specialty_id INT NOT NULL,
FOREIGN KEY (vet_id) REFERENCES vets(id),
FOREIGN KEY (specialty_id) REFERENCES specialties(id),
CONSTRAINT unique_ids UNIQUE (vet_id,specialty_id)
UNIQUE (vet_id, specialty_id)
);



CREATE TABLE IF NOT EXISTS types (
id SERIAL,
name VARCHAR(80),
CONSTRAINT pk_types PRIMARY KEY (id)
id SERIAL PRIMARY KEY,
name VARCHAR(80)
);

CREATE INDEX IF NOT EXISTS idx_types_name ON types (name);

ALTER SEQUENCE types_id_seq RESTART WITH 100;

CREATE TABLE IF NOT EXISTS owners (
id SERIAL,
id SERIAL PRIMARY KEY,
first_name VARCHAR(30),
last_name VARCHAR(30),
address VARCHAR(255),
city VARCHAR(80),
telephone VARCHAR(20),
CONSTRAINT pk_owners PRIMARY KEY (id)
telephone VARCHAR(20)
);

CREATE INDEX IF NOT EXISTS idx_owners_last_name ON owners (last_name);

ALTER SEQUENCE owners_id_seq RESTART WITH 100;


CREATE TABLE IF NOT EXISTS pets (
id SERIAL,
id SERIAL PRIMARY KEY,
name VARCHAR(30),
birth_date DATE,
type_id INT NOT NULL,
owner_id INT NOT NULL,
FOREIGN KEY (owner_id) REFERENCES owners(id),
FOREIGN KEY (type_id) REFERENCES types(id),
CONSTRAINT pk_pets PRIMARY KEY (id)
FOREIGN KEY (type_id) REFERENCES types(id)
);

CREATE INDEX IF NOT EXISTS idx_pets_name ON pets (name);

ALTER SEQUENCE pets_id_seq RESTART WITH 100;


CREATE TABLE IF NOT EXISTS visits (
id SERIAL,
id SERIAL PRIMARY KEY,
pet_id INT NOT NULL,
visit_date DATE,
description VARCHAR(255),
FOREIGN KEY (pet_id) REFERENCES pets(id),
CONSTRAINT pk_visits PRIMARY KEY (id)
FOREIGN KEY (pet_id) REFERENCES pets(id)
);

ALTER SEQUENCE visits_id_seq RESTART WITH 100;
Copy link
Member

Choose a reason for hiding this comment

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

question: I'm not aware of the potential side effects of removing the sequence reset. Did you?

Copy link
Author

Choose a reason for hiding this comment

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

Upon further testing, I found that the code currently submitted with all the changes does not work correctly: creating new objects fails. I did not check yet whether this is related to the primary key changes or if it has to do with the sequence reset. The error I am getting is of type "primary key exists" when inserting into the database.

Considering a major rework by aligning with the canonical version probably makes the most sense. I was also wondering whether the same issues exist for using the hql and mysql versions or whether this is specific to postgresql.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you @davosian for your tests.
Rewriting the PostgreSQL (and probably MySQL) DDL and DML scripts to be idempotent seems like a good idea. @dsyer did it on the canonical version.
Would you like to contribute to this extension?

HSQLDB and H2 are used as in-memory databases. Schema and data are re-created each time the application is started. So we don't need to rework them.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @arey , unfortunately I currently do not have the capacity to move forward with more involved contributions (besides being far from a Spring expert). I'd be happy to test any improvements with my current setup, though. I mainly set up a docker compose stack with postgresql, petclinic rest and petclinic angular.


CREATE TABLE IF NOT EXISTS users (
username VARCHAR(20) NOT NULL ,
password VARCHAR(20) NOT NULL ,
enabled boolean NOT NULL DEFAULT true ,
CONSTRAINT pk_users PRIMARY KEY (username)
username VARCHAR(20) NOT NULL PRIMARY KEY,
password VARCHAR(20) NOT NULL,
enabled BOOLEAN NOT NULL DEFAULT true
);

CREATE TABLE IF NOT EXISTS roles (
id SERIAL,
username varchar(20) NOT NULL,
role varchar(20) NOT NULL,
CONSTRAINT pk_roles PRIMARY KEY (id),
FOREIGN KEY (username) REFERENCES users (username)
);

ALTER TABLE roles ADD CONSTRAINT uni_username_role UNIQUE (role,username);
ALTER SEQUENCE roles_id_seq RESTART WITH 100;
id SERIAL PRIMARY KEY,
username VARCHAR(20) NOT NULL,
role VARCHAR(20) NOT NULL,
FOREIGN KEY (username) REFERENCES users (username),
UNIQUE (role, username)
);