Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

db:create erroneously states database already exists when connecting to timescaledb #1693

Closed
jhogendorn opened this issue Feb 6, 2023 · 5 comments · Fixed by #1704
Closed
Labels
Milestone

Comments

@jhogendorn
Copy link

How Shlink is set up

  • Shlink Version: stable
  • PHP Version: docker provided
  • How do you serve Shlink: Docker image
  • Database engine used: PostgreSQL

Summary

When connecting to a timescaledb postgresql instance the db:create cli function states the database already exists, which it does not. db:migrate then fails due to missing tables.

Expected behavior

db:create would create the tables

How to reproduce

Run the stable container pointing at a timescaledb (pg14) instance.

I suspect its seeing something in the non public schemas thats causing it to stop? timescaledb creates a number of other schemas alongside public.

@jhogendorn jhogendorn added the bug label Feb 6, 2023
@acelaya
Copy link
Member

acelaya commented Feb 6, 2023

What Shlink does in order to determine if the database exists is checking if any table which is not the migrations table itself, already exists in the database.

I have considered a few times changing the approach, as it's a bit naive, because the database could have other tables (which is probably what is happening here), but checking exactly for Shlink tables has some considerations:

  • Should it check all Shlink tables? Just some? If so, which ones?
  • Checking all Shlink tables implies having a list hardcoded somewhere, which can quickly become obsolete and cause other problems in the future. I haven't found a way in which it can be inferred from other elements where the tables are handled.

@acelaya acelaya added this to Shlink Feb 6, 2023
@acelaya acelaya added this to the 3.5.2 milestone Feb 6, 2023
@acelaya acelaya moved this to Todo 🗒️ in Shlink Feb 6, 2023
@jhogendorn
Copy link
Author

Yeah its a tricky one.

I think the interesting thing in this case is that in the public schema, there are no other tables. So its doing its check for tables in the other schemas. if we just restrict the scope to public this logic will be correct again.

@acelaya
Copy link
Member

acelaya commented Feb 7, 2023

Yeah, that's a good point. I will check if that's actually what's happening.

@acelaya acelaya moved this from Todo 🗒️ to In Progress 📝 in Shlink Feb 10, 2023
@acelaya acelaya moved this from In Progress 📝 to Todo 🗒️ in Shlink Feb 10, 2023
@acelaya acelaya moved this from Todo to In Progress in Shlink Feb 12, 2023
@acelaya
Copy link
Member

acelaya commented Feb 15, 2023

I have found a way to make Shlink check for its actual tables only, in a dynamic way (I don't need to change anything if the tables change, more are added, etc), which should cover this problem, as foreign tables will not affect this command anymore.

@acelaya acelaya moved this from In Progress to In review in Shlink Feb 15, 2023
@github-project-automation github-project-automation bot moved this from In review to Done in Shlink Feb 15, 2023
@acelaya
Copy link
Member

acelaya commented Feb 17, 2023

Shlink 3.5.2 was released yesterday, including this fix.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants