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.

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

Support CockroachDB #1404

Open
wodka opened this issue Oct 21, 2022 · 14 comments · May be fixed by #1405
Open

Support CockroachDB #1404

wodka opened this issue Oct 21, 2022 · 14 comments · May be fixed by #1405
Labels
enhancement New feature or request

Comments

@wodka
Copy link

wodka commented Oct 21, 2022

Hi :)

I've just setup Tolgee with CockroachDB (it's made to run without masters on kuberenets). With the initial Setup there were only 3 minor errors with the following migrations:

  • 1626957302295-1 (using syntax is not yet supported in CRDB - but I think will work out of the box in 23.1)
  • 1637177424380-0 (changing the length of the key - I think will also work out of the box in 23.1)
  • 1661361670848-3 the only real problem - as it will not support triggers

is there perhaps a way to generate the api keys without the triggers?

@JanCizmar
Copy link
Contributor

Hi there!

You don’t need the triggers. It was there just to cover the time, when there were running instances with multiple different versions. Feel free to submit PR disabling this migration for CockroachDB.

@wodka
Copy link
Author

wodka commented Oct 21, 2022

Neat :D I was assuming it is still needed because of the query that is being executed: https://github.com/tolgee/tolgee-platform/blob/main/backend/data/src/main/resources/db/changelog/hideApiKeyMigration.sql

can you point me in some direction to disable certain migrations?

@wodka
Copy link
Author

wodka commented Oct 21, 2022

:/ ok found another error: org.postgresql.util.PSQLException: ERROR: unknown signature: to_char(timestamp, string)

Hopefully would be added with cockroachdb/cockroach#3781 (but no clue when)

https://github.com/tolgee/tolgee-platform/blob/main/backend/data/src/main/kotlin/io/tolgee/repository/activity/ActivityRevisionRepository.kt#L64 here is the query with the function.

Would it be possible to change it to (not sure how to add the typecast in that query)

select 
  count(ar.id) as count,
  ar.timestamp::date as date
from 
  activity_revision ar
where 
  ar.project_id = 1
group by date
order by date

# Now:
# select count(ar.id) as count, function('to_char', ar.timestamp, 'yyyy-MM-dd') as date

# New? (::date would also format it in iso so the output would be identical if this works)
# select count(ar.id) as count, ar.timestamp::date as date

@JanCizmar
Copy link
Contributor

JanCizmar commented Oct 21, 2022

 can you point me in some direction to disable certain migrations?

You can specify dbms in the schema.xml file by setting dbms attribute for sql statement. See db/changelog/schema.xml:1482. But I don't think this will be sufficient for you. However, there is other way. It seems like you can send some parameters to the liquibase chengelog and use them.

You can provide these params when the liquibase bean is created:

io.tolgee.configuration.LiquibaseConfiguration#liquibase

To actually disable the statements, you can use precondidions:
https://docs.liquibase.com/concepts/changelogs/preconditions.html

I actually never done this. So I am just guessing.

@JanCizmar
Copy link
Contributor

Would it be possible to change it to (not sure how to add the typecast in that query)

I don't really remember why it's there. Let me check... 😆

@JanCizmar
Copy link
Contributor

Seems like you would be able to create same result, by using different methods supported by CockroachDB.

extract(element: string, input: date) → float

Extracts element from input.Compatible elements: millennium, century, decade, year, isoyear, quarter, month, week, dayofweek, isodow, dayofyear, julian, hour, minute, second, millisecond, microsecond, epoch

@JanCizmar
Copy link
Contributor

JanCizmar commented Oct 21, 2022

select count(ar.id) as count, ar.timestamp::date as date

I've tested this and it doesn't work, But this works:

select count(ar.id) as count, cast(cast(ar.timestamp as date) as text) as date

@wodka
Copy link
Author

wodka commented Oct 21, 2022

with select count(ar.id) as count, cast(cast(ar.timestamp as date) as text) as date it would work for CRDB as well! :D if that is a possible change for you it would be great

@JanCizmar JanCizmar linked a pull request Oct 22, 2022 that will close this issue
3 tasks
@JanCizmar
Copy link
Contributor

I've created a PR. Let's se what the test result shows, but it looks like we will have to wait for CRDB 22.2.

@wodka
Copy link
Author

wodka commented Oct 31, 2022

@JanCizmar is there a way for me to get a dockerimage with the modifications? Then I could also run this and see if something else pops up

@JanCizmar
Copy link
Contributor

You can clone the repo and run ./gradlew docker It generates tolgee/tolgee image, which you can use as you wish. Let me know if it worked. :)

@wodka
Copy link
Author

wodka commented Oct 31, 2022

:/ was a bit tricky to get the correct image but I managed it.

Sadly there is an error popping up:

ERROR: unimplemented: the configuration setting "join_collapse_limit" is not supported

coming from:

em.createNativeQuery("SET join_collapse_limit TO 1").executeUpdate()

The simple way of commenting the line seems to work - but the comment indicates to me that it is important for postgres....

Update: using the correct branch should help - rebuilding right now

Update 2: yes! it works!

@JanCizmar
Copy link
Contributor

JanCizmar commented Nov 1, 2022

 Update: using the correct branch should help - rebuilding right now

Yeah, it's fixed here https://github.com/tolgee/tolgee-platform/pull/1405/files#diff-b7d4751a11611455d0032b401f73be0b04084e153680caabdeecfdb0c33673d0R346

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Mar 9, 2023
@JanCizmar JanCizmar added the enhancement New feature or request label Mar 9, 2023
@JanCizmar JanCizmar reopened this Mar 9, 2023
@JanCizmar JanCizmar removed the stale label Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants