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

[Bug report] jdbc-mysql catalogs failed on mysql 8.x driver #3269

Closed
kalencaya opened this issue May 5, 2024 · 19 comments · Fixed by #3294
Closed

[Bug report] jdbc-mysql catalogs failed on mysql 8.x driver #3269

kalencaya opened this issue May 5, 2024 · 19 comments · Fixed by #3294
Assignees
Labels
0.5.1 Release v0.5.1 0.6.0 Release v0.6.0 bug Something isn't working

Comments

@kalencaya
Copy link
Contributor

Version

main branch

Describe what's wrong

jdbc-mysql catalogs failed on mysql 8.x driver, but 5.x works well

Error message and/or stacktrace

error

How to reproduce

mount mysql 8.x driver to gravitino docker container on /root/gravitino/catalogs/jdbc-mysql/libs/mysql.jar and create jdbc-mysql catalog on gravitino ui

Additional context

No response

@kalencaya kalencaya added the bug Something isn't working label May 5, 2024
@jerryshao
Copy link
Contributor

@FANNG1 @yuqi1129 would you please help to check this?

@yuqi1129
Copy link
Contributor

yuqi1129 commented May 6, 2024

@FANNG1 @yuqi1129 would you please help to check this?

Got it, I will verify it today.

@yuqi1129
Copy link
Contributor

yuqi1129 commented May 6, 2024

@kalencaya
Can you provide more information about it? We have used mysql-connector-java-8.0.27.jar for some time and it seems that nothing goes wrong, what is the version of JDBC-driver you use?

@kalencaya
Copy link
Contributor Author

docker-compose.yml and jdbc properties

driver-class-name: com.mysql.cj.jdbc.Driver
jdbc-url: jdbc:mysql://mysql:3306/test
username: root
password: 123456
version: "3.8"

services:

  mysql:
    image: bitnami/mysql:8.0
    container_name: mysql
    environment:
      - TZ=Asia/Shanghai
      - MYSQL_ROOT_USER=root
      - MYSQL_ROOT_PASSWORD=123456
      - MYSQL_AUTHENTICATION_PLUGIN=mysql_native_password
      - MYSQL_DATABASE=test
    ports:
      - 3306:3306
    networks:
      - test

  gravitino:
    image: datastrato/gravitino:0.5.0
    container_name: gravitino
    ports:
      - 8090:8090
    restart: unless-stopped
    volumes:
      - ./mysql-connector-java-8.0.11.jar:/root/gravitino/catalogs/jdbc-mysql/libs/mysql.jar
    networks:
      - test

networks:
  test:
    driver: bridge

@kalencaya
Copy link
Contributor Author

@yuqi1129 as you can see, I'm using mysql-connector-java-8.0.11.jar

@kalencaya
Copy link
Contributor Author

企业微信截图_01c6e282-cda2-43ea-9608-3a2cf8431ed3

@yuqi1129
Copy link
Contributor

yuqi1129 commented May 6, 2024

@yuqi1129 as you can see, I'm using mysql-connector-java-8.0.11.jar

OK, let me give it a try.

@yuqi1129
Copy link
Contributor

yuqi1129 commented May 6, 2024

@kalencaya
It did have a problem, we will fix it quickly. If you are interested, you can work on it too.
image

@yuqi1129
Copy link
Contributor

yuqi1129 commented May 6, 2024

It seems that the JDBC metadata is not very mature and stable. I'm afraid this is not the only issue that exists. We have addressed several issues with JDBC metadata such as mixed schema and catalog, wildcard issues, and so on.
@jerryshao @FANNG1

@kalencaya
Copy link
Contributor Author

@kalencaya It did have a problem, we will fix it quickly. If you are interested, you can work on it too. image

I'm willing to do this and pull requests

@kalencaya
Copy link
Contributor Author

kalencaya commented May 6, 2024

This should be caused by JdbcTableOperations#getTables(Connection) method:

  protected ResultSet getTables(Connection connection) throws SQLException {
    final DatabaseMetaData metaData = connection.getMetaData();
    String databaseName = connection.getSchema();
    return metaData.getTables(databaseName, databaseName, null, JdbcConnectorUtils.getTableTypes());
  }

I'll pull requests to change it as follow, which works well on mysql and postgresql:

  protected ResultSet getTables(Connection connection) throws SQLException {
    final DatabaseMetaData metaData = connection.getMetaData();
    String catalogName = connection.getCatalog();
    String schemaName = connection.getSchema();
    return metaData.getTables(catalogName, schemaName, null, JdbcConnectorUtils.getTableTypes());
  }

@yuqi1129
Copy link
Contributor

yuqi1129 commented May 6, 2024

@kalencaya
Thanks for your catch, please help to verify if the changes you mentioned conflict with #3229 or whether is it a better solution than that in #3229.

@kalencaya
Copy link
Contributor Author

kalencaya commented May 6, 2024

@kalencaya Thanks for your catch, please help to verify if the changes you mentioned conflict with #3229 or whether is it a better solution than that in #3229.

yes

@kalencaya
Copy link
Contributor Author

@kalencaya Thanks for your catch, please help to verify if the changes you mentioned conflict with #3229 or whether is it a better solution than that in #3229.

#3229 is a better solution

@yuqi1129
Copy link
Contributor

yuqi1129 commented May 7, 2024

@kalencaya Thanks for your catch, please help to verify if the changes you mentioned conflict with #3229 or whether is it a better solution than that in #3229.

#3229 is a better solution

Do you mean that #3229 can resolve this issue? @kalencaya

@FANNG1
Copy link
Contributor

FANNG1 commented May 7, 2024

  protected ResultSet getTables(Connection connection) throws SQLException {
    final DatabaseMetaData metaData = connection.getMetaData();
    String catalogName = connection.getCatalog();
    String schemaName = connection.getSchema();
    return metaData.getTables(catalogName, schemaName, null, JdbcConnectorUtils.getTableTypes());
  }

I prefer this solution than #3229 , because it's more general

@kalencaya
Copy link
Contributor Author

kalencaya commented May 7, 2024

  protected ResultSet getTables(Connection connection) throws SQLException {
    final DatabaseMetaData metaData = connection.getMetaData();
    String catalogName = connection.getCatalog();
    String schemaName = connection.getSchema();
    return metaData.getTables(catalogName, schemaName, null, JdbcConnectorUtils.getTableTypes());
  }

I prefer this solution than #3229 , because it's more general

this relies on jdbc driver api then appears more general.

but different jdbc driver implementations have some difference, they may have a nonstandard jdbc driver implementation.

actually, some dms (database management system) 、sql editor server or orm frameworks prefer to retrieve jdbc metadata through database internal metadata store tables not jdbc driver. more complicated than jdbc driver api way

I'm affected by that and prefers to not rely too much on jdbc driver api.

we may can't solve all jdbc metadata through jdbc driver api and have to query database internal metadata tables

just a little opinion

@yuqi1129
Copy link
Contributor

yuqi1129 commented May 7, 2024

this relies on jdbc driver api then appears more general.

but different jdbc driver implementations have some difference, they may have a nonstandard jdbc driver implementation.

actually, some dms (database management system) 、sql editor server or orm frameworks prefer to retrieve jdbc metadata through database internal metadata store tables not jdbc driver. more complicated than jdbc driver api way

I'm affected by that and prefers to not rely too much on jdbc driver api.

we may can't solve all jdbc metadata through jdbc driver api and have to query database internal metadata tables

just a little opinion

Please go ahead if it looks more elegant than #3229. #3229 will be modified based on your changes after your PR is merged.

@kalencaya
Copy link
Contributor Author

this relies on jdbc driver api then appears more general.
but different jdbc driver implementations have some difference, they may have a nonstandard jdbc driver implementation.
actually, some dms (database management system) 、sql editor server or orm frameworks prefer to retrieve jdbc metadata through database internal metadata store tables not jdbc driver. more complicated than jdbc driver api way
I'm affected by that and prefers to not rely too much on jdbc driver api.
we may can't solve all jdbc metadata through jdbc driver api and have to query database internal metadata tables
just a little opinion

Please go ahead if it looks more elegant than #3229. #3229 will be modified based on your changes after your PR is merged.

I'll pull requests recently and not block your subsequent works

yuqi1129 pushed a commit that referenced this issue May 9, 2024
… cause list tables error on some jdbc 8.x driver (#3294)

<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[#123] feat(operator): support xxx"
     - "[#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

replace `DatabaseMetaData#getSchema` by `DatabaseMetaData#getCatalog`
and provide a more general way to get tables through jdbc driver

### Why are the changes needed?

jdbc driver `DatabaseMetaData#getSchema` method return null causes list
tables failure on some jdbc mysql 8.x driver
(mysql-connector-java-8.0.11).

Fix: #3269

### Does this PR introduce _any_ user-facing change?

not

### How was this patch tested?

(Please test your changes, and provide instructions on how to test it:
1. If you add a feature or fix a bug, add a test to cover your changes.
2. If you fix a flaky test, repeat it for many times to prove it works.)

---------

Co-authored-by: wangqi <wangqi@xinxuan.net>
@yuqi1129 yuqi1129 added 0.5.1 Release v0.5.1 0.6.0 Release v0.6.0 labels May 9, 2024
github-actions bot pushed a commit that referenced this issue May 9, 2024
… cause list tables error on some jdbc 8.x driver (#3294)

<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[#123] feat(operator): support xxx"
     - "[#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

replace `DatabaseMetaData#getSchema` by `DatabaseMetaData#getCatalog`
and provide a more general way to get tables through jdbc driver

### Why are the changes needed?

jdbc driver `DatabaseMetaData#getSchema` method return null causes list
tables failure on some jdbc mysql 8.x driver
(mysql-connector-java-8.0.11).

Fix: #3269

### Does this PR introduce _any_ user-facing change?

not

### How was this patch tested?

(Please test your changes, and provide instructions on how to test it:
1. If you add a feature or fix a bug, add a test to cover your changes.
2. If you fix a flaky test, repeat it for many times to prove it works.)

---------

Co-authored-by: wangqi <wangqi@xinxuan.net>
yuqi1129 pushed a commit that referenced this issue May 10, 2024
… cause list tables error on some jdbc 8.x driver (#3316)

<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[#123] feat(operator): support xxx"
     - "[#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

replace `DatabaseMetaData#getSchema` by `DatabaseMetaData#getCatalog`
and provide a more general way to get tables through jdbc driver

### Why are the changes needed?

jdbc driver `DatabaseMetaData#getSchema` method return null causes list
tables failure on some jdbc mysql 8.x driver
(mysql-connector-java-8.0.11).

Fix: #3269

### Does this PR introduce _any_ user-facing change?

not

### How was this patch tested?

(Please test your changes, and provide instructions on how to test it:
1. If you add a feature or fix a bug, add a test to cover your changes.
2. If you fix a flaky test, repeat it for many times to prove it works.)

Co-authored-by: kalencaya <1942460489@qq.com>
Co-authored-by: wangqi <wangqi@xinxuan.net>
diqiu50 pushed a commit to diqiu50/gravitino that referenced this issue Jun 13, 2024
…n null cause list tables error on some jdbc 8.x driver (apache#3294)

<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[apache#123] feat(operator): support xxx"
     - "[apache#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[apache#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

replace `DatabaseMetaData#getSchema` by `DatabaseMetaData#getCatalog`
and provide a more general way to get tables through jdbc driver

### Why are the changes needed?

jdbc driver `DatabaseMetaData#getSchema` method return null causes list
tables failure on some jdbc mysql 8.x driver
(mysql-connector-java-8.0.11).

Fix: apache#3269

### Does this PR introduce _any_ user-facing change?

not

### How was this patch tested?

(Please test your changes, and provide instructions on how to test it:
1. If you add a feature or fix a bug, add a test to cover your changes.
2. If you fix a flaky test, repeat it for many times to prove it works.)

---------

Co-authored-by: wangqi <wangqi@xinxuan.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.5.1 Release v0.5.1 0.6.0 Release v0.6.0 bug Something isn't working
Projects
None yet
4 participants