Conversation
jessepeterson
left a comment
jessepeterson
left a comment
There was a problem hiding this comment.
Thanks for this! Just an initial/quick review. I want to take a closer look at this — especially the "conditional" queries, but I know you left it as draft just for that. :) Thanks again!
storage/mysql/query.sql
Outdated
| SELECT token_update_tally FROM enrollments WHERE id = ?; | ||
|
|
||
| -- name: StoreUserAuthenticateInitial :exec | ||
| INSERT INTO users |
There was a problem hiding this comment.
MySQL 8.0.19 deprecated the former ON DUPLICATE KEY syntax for the newer column reference style. However sqlc-dev/sqlc#2789 was filed because sqlc (actually the upstream SQL parser) does not support this syntax. This is why most of the Nano projects keep these queries in particular manually specified until sqlc supports the newer syntax.
The use of VALUES() to refer to the new row and columns is deprecated beginning with MySQL 8.0.20, and is subject to removal in a future version of MySQL. Instead, use row and column aliases, as described in the next few paragraphs of this section.
storage/mysql/query.sql
Outdated
| user_authenticate_at = CURRENT_TIMESTAMP; | ||
|
|
||
| -- name: StoreUserAuthenticateDigest :exec | ||
| INSERT INTO users |
There was a problem hiding this comment.
See other note about deprecated SQL syntax.
storage/mysql/query.sql
Outdated
| user_authenticate_digest_at = CURRENT_TIMESTAMP; | ||
|
|
||
| -- name: StoreAuthenticate :exec | ||
| INSERT INTO devices |
There was a problem hiding this comment.
See other note about deprecated SQL syntax.
| } | ||
| _, err := s.db.ExecContext( | ||
| r.Context(), ` | ||
| INSERT INTO devices |
There was a problem hiding this comment.
Per other note about deprecated SQL syntax this query will want to remain here.
| } | ||
| _, err := s.db.ExecContext( | ||
| r.Context(), ` | ||
| INSERT INTO users |
There was a problem hiding this comment.
Per other note about deprecated SQL syntax this query will want to remain here.
Converted storeDeviceTokenUpdate to sqlc, unfortunately that means 2 statements cause original 'built' the sql conditionally. Add sqlc.yaml config
csandrew-dev
force-pushed
the
feature/sqlc
branch
from
905dbd7 to
5c64c19
Compare
|
That commit (5c64c19) should remove the deprecated queries from the sqlc implementation and reverted them to the original. I believe the StoreDeviceTokenUpdate query was fine so I kept that in sqlc. |
|
This is looking overall good. Have you had a chance to run it and enroll do any enrollment testing devices? I think @bsoudan wants to explore more automated testing. I thought I approved you for GH actions, but I clicked that just now. Should start getting feedback from at least that.
Yep, the way you split it between two queries is a fine solution to sqlc's lack of "composable" queries. |
Signed-off-by: andrew s <andrew@openedmac.com>
Signed-off-by: andrew s <andrew@openedmac.com>
Signed-off-by: andrew s <andrew@openedmac.com>
Signed-off-by: andrew s <andrew@openedmac.com>
Signed-off-by: andrew s <andrew@openedmac.com>
Signed-off-by: andrew s <andrew@openedmac.com>
Signed-off-by: andrew s <andrew@openedmac.com>
I have to cast types because in schema.sql cert_pem and key_pem are TEXT, but when referenced in code are byte[] so sqlc generates them as strings in code. I think changing them to BLOBs in the schema would "fix" this, but not sure about the unintended changes that could make. Signed-off-by: andrew s <andrew@openedmac.com>
IsPushCertStale query returns an int32, so I have to cast. Signed-off-by: andrew s <andrew@openedmac.com>
function not used anymore because of sqlc usage. Signed-off-by: andrew s <andrew@openedmac.com>
2 participants
Making a draft pr so changes/edits can be requested and made before I convert the rest of the sql queries to use sqlc.
I believe these are correct but I haven't used sqlc before. (I am now realizing I could've converted the postgresql functions aswell 😅)
Partially related to issue #64, but as Jesse was saying, sqlc doesn't support for table prefixes. It has been mentioned in a few sqlc PRs, issues, and discussions.
Let me know what you think.