Skip to content

Revert "Add FixedString to PostgreSQL mapping" #3670

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

Merged
merged 1 commit into from
Apr 15, 2025

Conversation

pamarcos
Copy link
Member

@pamarcos pamarcos commented Apr 14, 2025

Summary

Revert #3489 due to ClickHouse/ClickHouse#79108

PostgreSQL's CHARACTER type stores number of characters, while ClickHouse's FixedString stores number of bytes. Thus, that 1:1 mapping doesn't work for all encodings. That's the reason it wasn't done for MySQL. Since those types mean different things, I think it's rather misleading to map one to the other. The safest option is to map CHARACTER types that mean characters to String instead.

e.g. there might be a Japanese word of 4 characters but that uses 18 bytes

Checklist

Copy link

vercel bot commented Apr 14, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
clickhouse-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 14, 2025 9:03am
clickhouse-docs-ru ❌ Failed (Inspect) Apr 14, 2025 9:03am
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
clickhouse-docs-jp ⬜️ Ignored (Inspect) Apr 14, 2025 9:03am
clickhouse-docs-zh ⬜️ Ignored (Inspect) Apr 14, 2025 9:03am

@pamarcos
Copy link
Member Author

pamarcos commented Apr 15, 2025

@Blargian any idea why the Deployment for Vercel failed? Is there any way to re-trigger that?
I don't have an account in Vercel and even if I did, I'm not sure I'd have permissions to see and re-trigger

@Blargian
Copy link
Member

@pamarcos the translation sites are not deployed on every commit to main so they get out of sync. I can see it's failing on an unrelated change. Actually Vercel should not even try to deploy the Russian translation docs unless a change was made to those docs in this PR, so it seems something is not working as it should. Will take a look.

Merging this.

@Blargian Blargian merged commit 9608b88 into main Apr 15, 2025
11 of 12 checks passed
@pamarcos
Copy link
Member Author

@pamarcos the translation sites are not deployed on every commit to main so they get out of sync. I can see it's failing on an unrelated change. Actually Vercel should not even try to deploy the Russian translation docs unless a change was made to those docs in this PR, so it seems something is not working as it should. Will take a look.

Merging this.

Thanks! 🤗
I did change the Russian doc slightly, though
image

@pamarcos pamarcos deleted the revert-add-fixedstring-to-postgresql-mapping branch April 15, 2025 08:16
@Blargian
Copy link
Member

@pamarcos the translation sites are not deployed on every commit to main so they get out of sync. I can see it's failing on an unrelated change. Actually Vercel should not even try to deploy the Russian translation docs unless a change was made to those docs in this PR, so it seems something is not working as it should. Will take a look.
Merging this.

Thanks! 🤗 I did change the Russian doc slightly, though image

Woops, missed that. No problem, I'll re-run translations for that page in a follow up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants