Mysql integration in Canyon #45

0nSystem · 2023-07-22T11:24:58Z

Hi guys, I am opening this PR as I want to do mysql integration. I open the PR in draft mode to be able to consult details and to be able to make a periodical follow-up of my progress.

Greetings ZeroDayCode.

…d data

docker/mysql/create_tables.sql

TheRustifyer · 2023-07-22T11:41:29Z

Hi guys, I am opening this PR as I want to do mysql integration. I open the PR in draft mode to be able to consult details and to be able to make a periodical follow-up of my progress.

Greetings ZeroDayCode.

I am happy to see you contributing for the very first time to the Canyon's codebase. I am looking forward to see how the implementation of MySql fits in our project :)

…d implement solution

…r and chrono:Date<OffSet>

0nSystem · 2023-08-12T11:18:51Z

Good morning,
I am starting to create test runs of select and update macros, without starting in querybuilder.
I receive syntax errors on certain occasions, an example is in the names of the columns due to a quotation marks, in case of sql server does not affect, the upper or lower case, in case mysq_async does not allow me to keep them, and in case of postgresql if it can be the case and can be problematic, so I comment this problem to obtain a solution as effective as possible and not a horrible patch without having analyzed together.

In the last commit is changed the format of columns of the update, since I was testing. But it should not exist and I understand that it is not a solution to support postgresql.

Greetings.

TheRustifyer · 2023-08-13T22:46:11Z

I don't understand what's the problem with posgres. Can you post the errores, problems or related code?

By the way, I am probably not understanding correctly your problem, but you not be messing up with posgres code.

Whatever alternative you need in MySQL, should be completly independent from postgresql code.

Are you maybe talking about the query macros? The approach is typically that postgre SQL is our base crate, so, whenever you need to do something with the queries, you must replace the problematic syntax in the executor, like we did in sqlserver.

This solution is far from perfect in terms of performance, but is the good one ATM to keep the queries code completly decoupled from one driver vendor to another, si people can come and implement new drivers, like your case with MySQL.

By the way, we're open to upgrade our codebase, so we will appreciate any idea

TheRustifyer · 2023-08-13T22:48:02Z

Good morning,
I am starting to create test runs of select and update macros, without starting in querybuilder.
I receive syntax errors on certain occasions, an example is in the names of the columns due to a quotation marks, in case of sql server does not affect, the upper or lower case, in case mysq_async does not allow me to keep them, and in case of postgresql if it can be the case and can be problematic, so I comment this problem to obtain a solution as effective as possible and not a horrible patch without having analyzed together.

In the last commit is changed the format of columns of the update, since I was testing. But it should not exist and I understand that it is not a solution to support postgresql.

Greetings.

@gbm25 by the way, you looked the upper-lower case standarization within Canyon for the queries. Can you help @0nSystem with its problem?

gbm25 · 2023-08-14T17:20:25Z

@gbm25 by the way, you looked the upper-lower case standarization within Canyon for the queries. Can you help @0nSystem with its problem?

If I remember correctly, by default we used the lowercase name for both table and column names, unless specified otherwise by the user.

Of course, to support both cases, the queries have to be prepared to search for those names taking into account that the names can contain uppercase letters because the table was created that way.

As I don't know what is the concrete problem of @0nSystem I will expose a small summary of the things related to table and column names in the different databases.

Identifier Behavior in Different Databases

PostgreSQL:

Identifiers that are not double-quoted are folded to lowercase.
Identifiers that are double-quoted retain their case and can be either reserved keywords or contain characters not generally allowed by the syntax rules.
Examples:

Click to see examples

Table Creation:

CREATE TABLE "TEST" (
    "COLUMN1" VARCHAR(255),
    "COLUMN2" INT,
    "COLUMN3" DATE,
    "COLUMN4" BOOLEAN
);

Sample Query:

INSERT INTO "TEST" ("COLUMN1", "COLUMN2", "COLUMN3", "COLUMN4") VALUES ('Sample', 123, '2023-08-14', TRUE);
SELECT * FROM "TEST" WHERE "COLUMN1" = 'Sample';

Reference documentation

Microsoft SQL Server:

By default, SQL Server identifiers are case-insensitive.
The SET QUOTED_IDENTIFIER option allows SQL Server to follow ISO rules regarding quotation mark delimiting identifiers and literal strings.
When SET QUOTED_IDENTIFIER is ON, identifiers can be delimited by double quotation marks (" ") and can be either reserved keywords or contain characters not generally allowed by the syntax rules.
Examples:

Click to see examples

Table Creation:

CREATE TABLE `TEST` (
    `COLUMN1` VARCHAR(255),
    `COLUMN2` INT,
    `COLUMN3` DATE,
    `COLUMN4` BOOLEAN
);

Sample Query:

INSERT INTO `TEST` (`COLUMN1`, `COLUMN2`, `COLUMN3`, `COLUMN4`) VALUES ('Sample', 123, '2023-08-14', TRUE);
SELECT * FROM `TEST` WHERE `COLUMN1` = 'Sample';

Reference documentation

MySQL:

Table names and column names are case-insensitive on Windows and case-sensitive on Unix-based systems by default.
Backticks (``) are used to quote identifiers, allowing for reserved words or special characters in names.
Examples:

Click to see examples

Table Creation:

SET QUOTED_IDENTIFIER ON;
CREATE TABLE "TEST" (
    "COLUMN1" VARCHAR(255),
    "COLUMN2" INT,
    "COLUMN3" DATE,
    "COLUMN4" BIT
);

Sample Query:

INSERT INTO "TEST" ("COLUMN1", "COLUMN2", "COLUMN3", "COLUMN4") VALUES ('Sample', 123, '2023-08-14', 1);
SELECT * FROM "TEST" WHERE "COLUMN1" = 'Sample';

Reference documentation

Given the above, it is clear that we were able to use the same standard with PostgreSQL and Microsoft SQL Server, but it is not going to be possible for MySQL.
In the migrations this is not a problem because they are differentiated, in the rest of the code ... I'm not sure because I don't touch it so much.

It can also be that not all queries follow this format in Canyon-SQL, because when it was implemented in Microsoft SQL Server it gave a lot of problems and many times we did things a little bit blindly.

Anyway, if you have any doubt with the queries, let me know which one is failing or which part of the code is giving you problems and we will see it.

0nSystem · 2023-08-14T18:47:34Z

@gbm25 by the way, you looked the upper-lower case standarization within Canyon for the queries. Can you help @0nSystem with its problem?

If I remember correctly, by default we used the lowercase name for both table and column names, unless specified otherwise by the user.

Of course, to support both cases, the queries have to be prepared to search for those names taking into account that the names can contain uppercase letters because the table was created that way.

As I don't know what is the concrete problem of @0nSystem I will expose a small summary of the things related to table and column names in the different databases.

Identifier Behavior in Different Databases

PostgreSQL:
* Identifiers that are **not double-quoted** are folded to lowercase.

* Identifiers that **are double-quoted** retain their case and can be either reserved keywords or contain characters not generally allowed by the syntax rules.

* Examples:
Click to see examples
* [Reference documentation](https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS)
Microsoft SQL Server:
* By default, SQL Server identifiers are **case-insensitive**.

* The `SET QUOTED_IDENTIFIER` option allows SQL Server to follow ISO rules regarding quotation mark delimiting identifiers and literal strings.

* When `SET QUOTED_IDENTIFIER` is `ON`, identifiers can be delimited by **double quotation marks** (`" "`) and can be either reserved keywords or contain characters not generally allowed by the syntax rules.

* Examples:
Click to see examples
* [Reference documentation](https://docs.microsoft.com/en-us/sql/t-sql/statements/set-quoted-identifier-transact-sql?view=sql-server-ver15)
MySQL:
* Table names and column names are **case-insensitive on Windows** and **case-sensitive on Unix-based systems** by default.

* **Backticks** (`` `` ``) are used to quote identifiers, allowing for reserved words or special characters in names.

* Examples:
Click to see examples
* [Reference documentation](https://dev.mysql.com/doc/refman/8.0/en/identifiers.html)
Given the above, it is clear that we were able to use the same standard with PostgreSQL and Microsoft SQL Server, but it is not going to be possible for MySQL. In the migrations this is not a problem because they are differentiated, in the rest of the code ... I'm not sure because I don't touch it so much.

It can also be that not all queries follow this format in Canyon-SQL, because when it was implemented in Microsoft SQL Server it gave a lot of problems and many times we did things a little bit blindly.

Anyway, if you have any doubt with the queries, let me know which one is failing or which part of the code is giving you problems and we will see it.

Hello @gbm25 @Pyzyryab,
With this you solve my problem, since I was referring to the name of standardization of the table and columns of postgres interferes with the implementation of mysql, but it is not a problem. I can solve it as a patch but I think we should look for a better way, than manipulating the query at the end prior to execution.

…r map

…and investigate get last insert in mysql_async

…se_connector.rs

… pattern in crud.rs

…tialization mssql

…ion count

damibazarra · 2023-10-13T21:31:14Z

@0nSystem is there any progress on this? I have a personal project in mind with some friends and we would like to do some testing with this innovative implementation. Great job!

0nSystem · 2023-10-14T13:37:11Z

@0nSystem is there any progress on this? I have a personal project in mind with some friends and we would like to do some testing with this innovative implementation. Great job!

Hello @damibazarra , Thanks

In principle everything is almost ready, currently the implementation is functional and you can start using it, I have to fix MysqlOperator and clippy errors.

0nSystem · 2023-10-23T18:20:23Z

The implementation is ready, if you see something that could be improved, do not hesitate to comment. @Pyzyryab @gbm25

TheRustifyer · 2023-11-03T19:05:25Z

The implementation is ready, if you see something that could be improved, do not hesitate to comment. @Pyzyryab @gbm25

Hey @0nSystem I reviewed briefly your changes, and except some minor things, everything looks fine. Even more, you cleaned some parts of the cfg macros that I liked.

Please, consider to carefully read the comments left as notes, and implement the changes whenever you feel ready. And by the way, you can change the PR mode to open, since we are already preparing for the next release, that will include the my_sql implementation.

Thanks for your job :)

0nSystem · 2023-11-04T08:51:39Z

Hey @Pyzyryab later today, I'll check the comments, and let you know when I'm ready to merge.

TheRustifyer · 2023-10-17T19:20:02Z

Cargo.toml

@@ -43,6 +45,8 @@ tokio = { version = "1.27.0", features = ["full"]  }
 tokio-util = { version = "0.7.4", features = ["compat"]  }
 tokio-postgres = { version = "0.7.2", features = ["with-chrono-0_4"] }
 tiberius = { version = "0.12.1", features = ["tds73", "chrono", "integrated-auth-gssapi"] }
+mysql_async = { version = "0.32.2" }
+mysql_common = { version = "0.30.6", features = [ "chrono" ]}


Sure that we need the two dependencies? Why do we need mysql_common? I guess that you've said because the format of the rows returned by the library, but we need to review if there's an approach that only need to rely on having the async crate

TheRustifyer · 2023-10-17T19:23:40Z

canyon_connection/src/canyon_database_connector.rs

+                    },
+                };
+
+                //TODO add options to optionals params in url


options to optionals?

TheRustifyer · 2023-10-17T19:24:57Z

canyon_crud/Cargo.toml

@@ -12,11 +12,17 @@ description.workspace = true
 [dependencies]
 tokio-postgres = { workspace = true, optional = true }
 tiberius = { workspace = true, optional = true }
+mysql_async = { workspace = true, optional = true }
+mysql_common = { workspace = true, optional = true }


Same as comment one, just to be sure

TheRustifyer · 2023-10-17T19:27:27Z

canyon_crud/src/bounds.rs

-    pub fn name(&self) -> &'_ str {
-        self.name
+    pub fn name(&self) -> &str {
+        &self.name
    }
    pub fn column_type(&self) -> &ColumnType {
        &self.type_


Feel free to remove the comments below, since they are there left by mistake

TheRustifyer · 2023-10-17T19:36:10Z

canyon_crud/src/bounds.rs

    type_: ColumnType,
 }
 impl<'a> Column<'a> {
-    pub fn name(&self) -> &'_ str {
-        self.name
+    pub fn name(&self) -> &str {


You shouldn't change things that aren't meant to be changed in this kind of PR.
The anonymous lifetime there is for one purpose, literally, to indicate an anonymous lifetime.
&'_

TheRustifyer · 2023-11-03T18:42:31Z

canyon_crud/src/crud.rs

+        let pp: &str = positional_param.as_str();
+        let pp_index = pp[1..] // param $1 -> get 1
+            .parse::<usize>()
+            .expect("Error parse mapped parameter to usized.")


Review English please

TheRustifyer · 2023-11-03T18:44:57Z

canyon_crud/src/crud.rs

+    stmt: &str,
+    params: &[&'_ dyn QueryParameter<'_>],
+    fn_parser: impl Fn(&&dyn QueryParameter<'_>) -> T,
+) -> Vec<T> {


This method is "unwrapping" (.expect(msg)), which leads to panic the process, instead of return with an error. And is made in a lot of places.

Please, implement a more curated version that handles properly the Result or Optional types, and return a Result<Vec, _> with the _ as a placeholder for the error that you need to return if it's the case

TheRustifyer · 2023-11-03T18:46:11Z

canyon_crud/src/query_elements/operators.rs

@@ -20,7 +22,7 @@ pub enum Comp {
 }

 impl Operator for Comp {
-    fn as_str(&self, placeholder_counter: usize) -> String {
+    fn as_str(&self, placeholder_counter: usize, _datasource_type: &DatabaseType) -> String {


_datasource_type: &DatabaseType isn't being used in the code, nor expected to be used

TheRustifyer · 2023-11-03T18:48:42Z

canyon_macros/src/lib.rs

-                        #(#init_field_values_sqlserver),*
-                    }
+    let tokens = quote! {
+        impl canyon_sql::crud::RowMapper<Self> for #ty {


Good job. You solved this implementation issue elegantly!

canyon_macros/src/query_operations/insert.rs

mysql-support start with docker configuration to up container and loa…

8e9a190

…d data

0nSystem self-assigned this Jul 22, 2023

TheRustifyer reviewed Jul 22, 2023

View reviewed changes

docker/mysql/create_tables.sql Show resolved Hide resolved

0nSystem and others added 6 commits July 22, 2023 20:22

mysql-support - first implementation workspace canyon_connection

adb63dc

mysql - change module bounds pending DateTime<OffSet> look problem an…

f99f3b3

…d implement solution

mysql-support canyon macros, pending trait implement FromValue in &st…

e067220

…r and chrono:Date<OffSet>

mysql-support count table implemented

e53d7c6

mysql-support - pending implement select test

bf21ca7

mysql-support craete reorder array params to use with mysql params

904e554

OnSystem and others added 14 commits August 21, 2023 19:49

mysql-support resolve quotes in queries and innecesary loop in reorde…

f3376fc

…r map

mysql-support select, update , and query_builder

b1da9bb

mysql-support correction test insert and delete mysql implementation …

b3164c6

…and investigate get last insert in mysql_async

develop remove imports not use

39bfa88

mysql-support added integration to returning primary key

9ce698c

mysql-support correction cfg features

9d46976

mysql-support remove comment to mysql implementation in canyon_databa…

7245b75

…se_connector.rs

mysql-support canyon_macro.rs added in feature migrations

8b3ecd6

mysql-support replace error with feature in canyon_macro and separate…

335fe0d

… pattern in crud.rs

changes: remove CanyonRowsMysql and implement mysql_async, repair ini…

2e9edb2

…tialization mssql

changes: corrections other test

1715b18

changes: insert and multiinsert match_rows

0871b40

changes: lib cargo_macros RowMapper

625672a

changes: select macro remove else if to generate tokenstream with act…

195bd77

…ion count

0nSystem added 2 commits October 14, 2023 17:55

changes: resolve clippy error format! in expect to create Regex

7647373

changes: resolve clippy error format! in expect to create Regex

8631628

TheRustifyer self-requested a review October 17, 2023 19:15

0nSystem marked this pull request as ready for review October 19, 2023 20:50

0nSystem requested a review from gbm25 as a code owner October 19, 2023 20:50

0nSystem added 2 commits October 20, 2023 00:32

changes: implement Operator by datasource type

144e9df

changes: added default datasources

9e4c91d

0nSystem added 2 commits December 2, 2023 21:57

changes: correction error feature specify in datasources.rs enum Auth

c51a00e

changes: correction cargo fmt

5c09a3f

TheRustifyer approved these changes Dec 10, 2023

View reviewed changes

TheRustifyer merged commit 13b2a98 into zerodaycode:main Dec 10, 2023
8 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mysql integration in Canyon #45

Mysql integration in Canyon #45

0nSystem commented Jul 22, 2023

TheRustifyer commented Jul 22, 2023

0nSystem commented Aug 12, 2023

TheRustifyer commented Aug 13, 2023

TheRustifyer commented Aug 13, 2023

gbm25 commented Aug 14, 2023

Table Creation:

Sample Query:

Table Creation:

Sample Query:

Table Creation:

Sample Query:

0nSystem commented Aug 14, 2023

Identifier Behavior in Different Databases

PostgreSQL:

Microsoft SQL Server:

MySQL:

damibazarra commented Oct 13, 2023

0nSystem commented Oct 14, 2023

0nSystem commented Oct 23, 2023

TheRustifyer commented Nov 3, 2023

0nSystem commented Nov 4, 2023

TheRustifyer Oct 17, 2023

TheRustifyer Oct 17, 2023

TheRustifyer Oct 17, 2023

TheRustifyer Oct 17, 2023

TheRustifyer Oct 17, 2023

TheRustifyer Nov 3, 2023

TheRustifyer Nov 3, 2023

TheRustifyer Nov 3, 2023

TheRustifyer Nov 3, 2023

Mysql integration in Canyon #45

Mysql integration in Canyon #45

Conversation

0nSystem commented Jul 22, 2023

TheRustifyer commented Jul 22, 2023

0nSystem commented Aug 12, 2023

TheRustifyer commented Aug 13, 2023

TheRustifyer commented Aug 13, 2023

gbm25 commented Aug 14, 2023

Identifier Behavior in Different Databases

PostgreSQL:

Table Creation:

Sample Query:

Microsoft SQL Server:

Table Creation:

Sample Query:

MySQL:

Table Creation:

Sample Query:

0nSystem commented Aug 14, 2023

Identifier Behavior in Different Databases

PostgreSQL:

Microsoft SQL Server:

MySQL:

damibazarra commented Oct 13, 2023

0nSystem commented Oct 14, 2023

0nSystem commented Oct 23, 2023

TheRustifyer commented Nov 3, 2023

0nSystem commented Nov 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment