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

fix(mysql): quote database name in CREATE DATABASE statement #149

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dakraus
Copy link

@dakraus dakraus commented Nov 28, 2022

In order to allow the full Unicode Basic Multilingual Plane,
the database name is now quoted in the CREATE DATABASE statement.

fixes #148

In order to allow the full Unicode Basic Multilingual Plane in the
database name, the name is now quoted in the CREATE DATABASE statement.

Signed-off-by: Daniel Kraus <daniel.kraus@kubermatic.com>
@brandond
Copy link
Contributor

Thank you for the contribution! Would you mind also fixing this for the other drivers as well?

@dakraus
Copy link
Author

dakraus commented Nov 29, 2022

Sure @brandond, I will check the other drivers to see if we this fix is needed elsewhere.

In order to allow the full Unicode Basic Multilingual Plane in the
database name, the name is now quoted in the CREATE DATABASE statement

Note that quoting an identifier also makes it case-sensitive, whereas
unquoted names are always folded to lower case.

Signed-off-by: Daniel Kraus <daniel.kraus@kubermatic.com>
@dakraus
Copy link
Author

dakraus commented Nov 29, 2022

@brandond I added another commit to fix this for the PostgreSQL driver. However this also means that the database names for the PostgreSQL driver are now case-sensitive according to the official documentation:

Quoting an identifier also makes it case-sensitive, whereas unquoted names are always folded to lower case.

Another thing I noticed while running the tests locally: The CREATE DATABASE statement is never executed for CockroachDB and kine fails during the schema creation with if you configure a non-existing database in the KINE_ENDPOINT

[cockroachdb-v20.2] time="2022-11-29T12:51:42Z" level=info msg="starting metrics server path /metrics"
[cockroachdb-v20.2] time="2022-11-29T12:51:42Z" level=info msg="Configuring postgres database connection pooling: maxIdleConns=2, maxOpenConns=0, connMaxLifetime=0s"
[cockroachdb-v20.2] time="2022-11-29T12:51:42Z" level=info msg="Configuring database table schema and indexes, this may take a moment..."
[cockroachdb-v20.2] time="2022-11-29T12:51:42Z" level=trace msg="SETUP EXEC : CREATE TABLE IF NOT EXISTS kine ( id SERIAL PRIMARY KEY, name VARCHAR(630), created INTEGER, deleted INTEGER, create_revision INTEGER, prev_revision INTEGER, lease INTEGER, value bytea, old_value bytea );"
[cockroachdb-v20.2] time="2022-11-29T12:51:42Z" level=fatal msg="building kine: pq: no database or schema specified"

@brandond
Copy link
Contributor

brandond commented Dec 5, 2022

Thanks! I take it the sqlite driver didn't need any changes?

However this also means that the database names for the PostgreSQL driver are now case-sensitive.

That seems like a fair tradeoff; I'll make sure that is mentioned in the release notes.

The CREATE DATABASE statement is never executed for CockroachDB

Hmm, the logs suggest that it is - are you saying that it actually is not? Or that the CREATE TABLE statement just silently fails?

@dakraus
Copy link
Author

dakraus commented Dec 6, 2022

Yes, the sqlite driver just works.

Regarding CockroachDB:
The CREATE DATABASE statement is obviously executed (as shown in the logs) and the tests are also passing in Drone, so I guess that something is wrong on my end. If I can reproduce this behavior, I would open another issue for that.
I have to correct my statement above - the CREATE DATABASE statement is actually not executed. I described this in #150.

@dereknola
Copy link
Contributor

Is this still relevant?

@brandond
Copy link
Contributor

It is, but I have been reluctant to merge it because of the change in behavior for Postgres.

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.

mysql: creation of database with hyphen fails
3 participants