ballerina-platform / ballerina-library

The Ballerina Library
https://ballerina.io/learn/api-docs/ballerina/
Apache License 2.0
137 stars 65 forks source link

Having driver version specific properties in mysql module gives errors #178

Closed anupama-pathirage closed 3 years ago

anupama-pathirage commented 4 years ago

Description:

Even though the docs [1] says it is recommended to use a MySQL driver version greater than 8.0.x, there can be version-specific properties introduced by each driver version, and ballerina msyql module can be only used with the newest versions of drivers it seems.

So the mysql module is tightly coupled with driver versions now.

- Are we going to release mysql modules for future driver versions? - Sometimes some properties make the current module unusable for even we switch between the drivers in the same major version. (Refer below example) - MySQL server version also has some impact on this (Ex : MySQL Connector/J 8.0 is highly recommended for use with MySQL Server 8.0, 5.7, and 5.6 [2]).

So having these version-specific options in our records, makes it complex to manage ballerina mysql module versions and it limits the usability of the module. (We have to fall back to jdbc module to get the work done properly)

Ex:

If the server is not configured to use SSL, we need to disable SSL and suppress the SSL errors before proceeding. To do that we need to set the property useSSL to false.

If the driver version is : (According to driver docs[3] )

8.0.12 and earlier: Use the property useSSL=false 8.0.13 and later: We can use sslMode

In current Ballerina mysql module, if we are using driver version 8.0.12 and earlier we don’t have a way to specify this useSSL property due to following reasons.

  1. It doesn’t support providing JDBC url directlly. So we can’t use the approach of appending the property to the jdbc url.
  2. The mysql db Options record [4] doesn’t provide the support for adding custom properties.
  3. Using ssl config [5] as below gives the following error.

Code:

import ballerina/sql;
import ballerina/io;
import ballerina/mysql;

public function main() returns error? {
    mysql:Options mySQLOptions = {
        ssl: {
            mode: mysql:SSL_PREFERRED
        }
    };

    mysql:Client|error dbClient =  new ("localhost", "root", "password", "testdb", 3306, mySQLOptions);
    if (dbClient is mysql:Client) {
        sql:ExecutionResult|error executeRes = dbClient->execute("CREATE TABLE IF NOT EXISTS Customers(customerId INTEGER, firstName  VARCHAR(300))");
        io:println(executeRes);
    } else {
        io:println(dbClient);
    }

    if (dbClient is mysql:Client) {
        error? closeOut = dbClient.close();
    }
}

Error:

error error in sql connector configuration: Property sslMode does not exist on target class com.mysql.cj.jdbc.MysqlDataSource

Solutions for the above issue:

  1. Change the mysql server-side configs to use ssl (Difficult in prod envs)
  2. Change the driver version to 8.0.13 and later. (Difficult in prod envs and still not sure whether another property can cause a similar issue)
  3. Fall back to jdbc module. It can be used in two ways to solve this.
    jdbc:Options mysqlOptions = {
        properties: {"useSSL": "false"}
    };

    jdbc:Client|error dbClient =  new ("jdbc:mysql://localhost:3306/testdb",
        "root", "password", mysqlOptions);

[1] https://ballerina.io/swan-lake/learn/api-docs/ballerina/mysql/index.html [2] https://dev.mysql.com/doc/connector-j/8.0/en/ [3] https://dev.mysql.com/doc/connector-j/8.0/en/connector-j-reference-using-ssl.html [4] https://ballerina.io/swan-lake/learn/api-docs/ballerina/mysql/records/Options.html [5] https://ballerina.io/swan-lake/learn/api-docs/ballerina/mysql/records/SSLConfig.html

Steps to reproduce:

Affected Versions:

OS, DB, other environment details and versions:

Related Issues (optional):

Suggested Labels (optional):

Suggested Assignees (optional):

lahirusamith commented 3 years ago

@daneshk @niveathika Need to select a solution out of those suggestions. WDYT?

daneshk commented 3 years ago

As a solution, we will change the API Docs recommending 8.0.13 and later and improve the error message to provide a meaningful message.