ClusterCockpit / cc-backend

Web frontend and API backend server for ClusterCockpit Monitoring Framework
https://www.clustercockpit.org
MIT License
14 stars 12 forks source link

SQL statement syntax #231

Closed aw32 closed 4 months ago

aw32 commented 7 months ago

Creating new tags always results in the following error (both per web interface and REST API):

Error 1054 (42S22): Unknown column '$1' in 'field list'

Further investigation lead me to the following line: https://github.com/ClusterCockpit/cc-backend/blob/280b16c11c1760e66f053fe24751712b69907b8e/internal/repository/tags.go#L62C25-L62C25

INSERT INTO tag (tag_type, tag_name) VALUES ($1, $2)

The sqlx package documentation says:

    MySQL uses the ? variant shown above
    PostgreSQL uses an enumerated $1, $2, etc bindvar syntax
    SQLite accepts both ? and $1 syntax
    Oracle uses a :name syntax

We are running cc-backend with MySQL (mariadb) and it seems the "$1" is passed verbatim to the server without inserting the value. Grepping through the code shows, that all three syntaxes are used in the code. I found the ":name" syntax in internal/repository/job.go, but still need to figure out if these statements are also problematic on our instance.

moebiusband73 commented 4 months ago

I used the squirrel query builder which automatically uses the correct syntax for placeholders. In my tests with a MariaDB test database this works now. The :name Syntax is used in NamedJobInsert, this is used for example in the init-db job import. I tested it against MariaDB and it didn't cause any issues.

moebiusband73 commented 4 months ago

We will convert raw SQL statements into query builder on the go from now on.

moebiusband73 commented 4 months ago

One more thing. There was a Bug in the initial database initialization.

For the tags table the autoincrement was missing. I just added this now, but this is of course not the right way to do it. I have to add a new migration. Question: How was your table initilalized?