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

MySQL support for Nessie(JDBC based) #8483

Merged
merged 9 commits into from
May 21, 2024

Conversation

vyj7
Copy link
Contributor

@vyj7 vyj7 commented May 8, 2024

Added support to use mySQL database for nessie.

@CLAassistant
Copy link

CLAassistant commented May 8, 2024

CLA assistant check
All committers have signed the CLA.

@adutra
Copy link
Contributor

adutra commented May 10, 2024

Hi @vyj7 thanks for contributing to Nessie! It turns out, I was myself planning to introduce support for MySQL. I'm afraid this will require a lot more changes compared to what you have in this PR. Are you willing to continue working on this (in which case, I could give you some pointers), or should I take it over from here? Just le me know :-) Thanks!

(Note: you also would need to sign our CLA.)

@vyj7
Copy link
Contributor Author

vyj7 commented May 10, 2024

@adutra Yes I would love to work on this. Can you guide me what else I am missing.If you can give me a small walkthrough or doc how code is structured i can pick up

@adutra
Copy link
Contributor

adutra commented May 13, 2024

@adutra Yes I would love to work on this. Can you guide me what else I am missing.If you can give me a small walkthrough or doc how code is structured i can pick up

@vyj7 awesome, but we need to sort out a few legal issues first. I will ping you when we're ready.

@adutra
Copy link
Contributor

adutra commented May 14, 2024

Hi @vyj7 we're ready to work on this.

We decided that it's not safe for Nessie to be distributed with the MySQL driver. But there is one thing we can do: offer support for MariaDB instead, and use the MariaDB driver to connect to either MariaDB or MySQL.

So, if you are still OK to work on this, here are the first steps:

  1. In gradle/libs.versions.toml, declare the MariaDB driver:
mariadb-java-client = { module = "org.mariadb.jdbc:mariadb-java-client", version = "3.3.3" }
  1. In gradle/license/allowed-licenses.json, declare the LGPL license:
    {
      "moduleLicense": "GNU Lesser General Public License Version 2.1"
    },
  1. In gradle/license/allowed-licenses.json add the following:
    { "bundleName" : "lgpl21", "licenseNamePattern" : "LGPL-2.1" },
  1. In versioned/storage/jdbc/build.gradle.kts, declare the MariaDB driver:
  intTestRuntimeOnly(libs.mariadb.java.client)
  1. Add support for MariaDB/MySQL in DatabaseSpecifics.MariaDBDatabaseSpecific – since I already worked on this a bit, I'm giving you my version. Note: I used the utf8mb4_bin collation because it works for both MySQL and MariaDB, but if you have a better one, no problem. Here it is:
  static class MariaDBDatabaseSpecific implements DatabaseSpecific {

    private static final String VARCHAR = "VARCHAR(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin";
    private static final String TEXT = "TEXT CHARACTER SET utf8mb4 COLLATE utf8mb4_bin";
    private static final String MYSQL_CONSTRAINT_VIOLATION_SQL_STATE = "23000";

    private final Map<JdbcColumnType, String> typeMap;
    private final Map<JdbcColumnType, Integer> typeIdMap;

    MariaDBDatabaseSpecific() {
      typeMap = new EnumMap<>(JdbcColumnType.class);
      typeIdMap = new EnumMap<>(JdbcColumnType.class);
      typeMap.put(JdbcColumnType.NAME, VARCHAR);
      typeIdMap.put(JdbcColumnType.NAME, Types.VARCHAR);
      typeMap.put(JdbcColumnType.OBJ_ID, VARCHAR);
      typeIdMap.put(JdbcColumnType.OBJ_ID, Types.VARCHAR);
      typeMap.put(JdbcColumnType.OBJ_ID_LIST, TEXT);
      typeIdMap.put(JdbcColumnType.OBJ_ID_LIST, Types.VARCHAR);
      typeMap.put(JdbcColumnType.BOOL, "BIT(1)");
      typeIdMap.put(JdbcColumnType.BOOL, Types.BIT);
      typeMap.put(JdbcColumnType.VARBINARY, "BLOB");
      typeIdMap.put(JdbcColumnType.VARBINARY, Types.BLOB);
      typeMap.put(JdbcColumnType.BIGINT, "BIGINT");
      typeIdMap.put(JdbcColumnType.BIGINT, Types.BIGINT);
      typeMap.put(JdbcColumnType.VARCHAR, VARCHAR);
      typeIdMap.put(JdbcColumnType.VARCHAR, Types.VARCHAR);
    }

    @Override
    public Map<JdbcColumnType, String> columnTypes() {
      return typeMap;
    }

    @Override
    public Map<JdbcColumnType, Integer> columnTypeIds() {
      return typeIdMap;
    }

    @Override
    public boolean isConstraintViolation(SQLException e) {
      return MYSQL_CONSTRAINT_VIOLATION_SQL_STATE.equals(e.getSQLState());
    }

    @Override
    public String wrapInsert(String sql) {
      return sql.replace("INSERT INTO", "INSERT IGNORE INTO");
    }
  }

Once you're done we can tackle the tests. Good luck :-)

@vyj7
Copy link
Contributor Author

vyj7 commented May 15, 2024

@adutra yes I think mariaDB driver should also work for mysql. I will work on this.

@adutra
Copy link
Contributor

adutra commented May 15, 2024

@vyj7 I think you accidentally closed this PR?

@vyj7
Copy link
Contributor Author

vyj7 commented May 16, 2024

I removed changes done for mysql. I will push changes on this branch only.

@vyj7 vyj7 reopened this May 16, 2024
@vyj7
Copy link
Contributor Author

vyj7 commented May 16, 2024

@adutra I couldn't find gradle/license/allowed-licenses.json and gradle/license/allowed-licenses.json in project

@adutra
Copy link
Contributor

adutra commented May 16, 2024

@adutra I couldn't find gradle/license/allowed-licenses.json and gradle/license/allowed-licenses.json in project

@vyj7 it's because your branch is out of date with the main branch. Let's ignore that for now and focus on the rest.

@adutra
Copy link
Contributor

adutra commented May 16, 2024

@vyj7 for the tests, here's what you need to do:

  1. In nessie-versioned-storage-jdbc-tests implement MariaDBBackendTestFactory et MySQLBackendTestFactory.
  2. In nessie-versioned-storage-jdbc implement the following tests:
    a. ITMariaDBBackendFactory
    b. ITMariaDBPersist
    c. ITMariaDBVersionStore
    d. ITMySQLBackendFactory
    e. ITMySQLPersist
    f. ITMySQLVersionStore

@snazy
Copy link
Member

snazy commented May 16, 2024

@vyj7 for the tests, here's what you need to do:

Also:
ITRestApiPersistMariaDB in nessie-quarkus
QuarkusTestProfilePersistMariaDB + MariaDBTestResourceLifecycleManager in nessie-quarkus-tests

@adutra
Copy link
Contributor

adutra commented May 17, 2024

@vyj7 for the tests, here's what you need to do:

Also: ITRestApiPersistMariaDB in nessie-quarkus QuarkusTestProfilePersistMariaDB + MariaDBTestResourceLifecycleManager in nessie-quarkus-tests

Not in this PR please :-) Let's tackle just the persist layer, the rest is much more involved and will be done in a follow-up PR.

vyj7 added 2 commits May 17, 2024 16:17
Support to detect JDBC backend for mysql and mariaDb
gradle/libs.versions.toml Outdated Show resolved Hide resolved
@vyj7
Copy link
Contributor Author

vyj7 commented May 17, 2024

@adutra how should i run this code in order to test it with a local mysql/maria-Db image.
I changed properties servers/quarkus-server/src/main/resources/application.properties
nessie.version.store.type=JDBC quarkus.datasource.db-kind=mariadb quarkus.datasource.username=user quarkus.datasource.password=password quarkus.datasource.jdbc.url=jdbc:mariadb://localhost:3302/my_database

and then ran task in nessie Quarkus - quarkusRun

@adutra
Copy link
Contributor

adutra commented May 17, 2024

@vyj7 you don't need to run the server, all you need to do is run the JDBC persistence tests:

./gradlew :nessie-versioned-storage-jdbc:build

If the build passes once you've added the new tests, then you are done :-)

gradle/libs.versions.toml Outdated Show resolved Hide resolved
Added TCs for maria DB and mySQL
@vyj7
Copy link
Contributor Author

vyj7 commented May 19, 2024

@adutra I have committed the TCs. All the testcases actually failed for mariaDB(TCs are failing) and mySQL(driver not found). I am debugging what are the causes of it. So far the test containers are spawning correctly.

@vyj7
Copy link
Contributor Author

vyj7 commented May 20, 2024

@adutra Caused by: java.lang.ClassNotFoundException: org.projectnessie.nessie.relocated.protobuf.UnsafeByteOperations
Do you know about this error? I directly ran one of the integration test to see if changes work

Modified MySQL testcontainer to use mariaDb driver
@adutra
Copy link
Contributor

adutra commented May 20, 2024

@adutra Caused by: java.lang.ClassNotFoundException: org.projectnessie.nessie.relocated.protobuf.UnsafeByteOperations Do you know about this error? I directly ran one of the integration test to see if changes work

Yes this happens sometimes in your IDE. You need to build the nessie-protobuf-relocated module:

 ./gradlew :nessie-protobuf-relocated:build

Copy link
Contributor

@adutra adutra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vyj7 You have a few test failures in AbstractTestJdbcBackendFactory.incompatibleTableSchema().

I think you can easily fix them by replacing all occurrences of VARCHAR with VARCHAR(255).

Modified AbstractTestJdbcBackendFactory varchar to varchar(255)
@vyj7
Copy link
Contributor Author

vyj7 commented May 20, 2024

@adutra All testcases passed after the changes

@adutra adutra marked this pull request as ready for review May 20, 2024 18:20
Copy link
Contributor

@adutra adutra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for your contribution @vyj7 !

@adutra adutra merged commit 39ec7d0 into projectnessie:main May 21, 2024
16 checks passed
@vyj7
Copy link
Contributor Author

vyj7 commented May 21, 2024

Pleasure is mine. Really enjoyed working on this. :-)

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.

None yet

4 participants