bugfix: fix the column TINYINT(1) in MySql being resolved to BIT type
#6068
Conversation
wangliang181230
requested review from
funky-eyes and
slievrly
and removed request for
funky-eyes
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #6068 +/- ##
=============================================
+ Coverage 48.86% 48.92% +0.05%
- Complexity 4182 4191 +9
=============================================
Files 793 795 +2
Lines 28039 28085 +46
Branches 3426 3432 +6
=============================================
+ Hits 13701 13740 +39
- Misses 12901 12905 +4
- Partials 1437 1440 +3
|
This reverts commit b00bb48.
rm-datasource/src/main/java/io/seata/rm/datasource/sql/struct/cache/MysqlTableMetaCache.java
Outdated
Show resolved
Hide resolved
...-mysql8/src/main/java/io/seata/rm/datasource/sql/struct/cache/Mysql8ColumnMetaProcessor.java
Show resolved
Hide resolved
| // JDBCType.DISTINCT, JDBCType.STRUCT etc... | ||
| field.setValue(holdSerialDataType(resultSet.getObject(i))); | ||
| } | ||
| loadFieldValue(field, resultSet, i); |
There was a problem hiding this comment.
这个方法都抽离出去独立了，不如把212行的代码也挪过去吧，增加一个dataType的入参
Since this method has been extracted and made independent, why not move the code from line 212 as well and add a dataType parameter
There was a problem hiding this comment.
我觉得没必要吧。对于 loadFieldValue 方法来说，它不应该负责 setDataType 的。
There was a problem hiding this comment.
但是loadfieldvalue中你又进行了一次getdatatype，这样合适吗？
But in loadFieldValue, you are doing another getDataType() call. Is that appropriate?
There was a problem hiding this comment.
loadFieldValue的实现，是通过dataType来判断，去加载value，这很合理啊。
但是loadFieldValue方法的调用方，单从方法名上看，就无法理解为什么要把setDataType放到这个方法中。
There was a problem hiding this comment.
loadFieldValue的实现，是通过dataType来判断，去加载value，这很合理啊。 但是loadFieldValue方法的调用方，单从方法名上看，就无法理解为什么要把setDataType放到这个方法中。
那你的方法名是不是应该修改下？你也说了loadFieldValue依赖dataType
Then should you modify your method name? You also mentioned that loadFieldValue depends on dataType.
There was a problem hiding this comment.
调用方，又不用关心它是怎么实现的，只要知道这个方法是加载value的就行了啊。
There was a problem hiding this comment.
你在定义一个接口的方法名的时候，你会去考虑实现方式来定方法名吗？肯定不会吧。
There was a problem hiding this comment.
调用方，又不用关心它是怎么实现的，只要知道这个方法是加载value的就行了啊。
谁在调用？除了这还有哪里？
Who is calling it? Are there any other places besides this?
There was a problem hiding this comment.
loadFieldValue 依赖dataType，结果也不进行传参，到里面了get一下，我理解不了这么写的写法。如果方法名觉得不直观可以改，loadfieldvalue只服务于buildRecords
| @@ -38,7 +38,7 @@ protected TableMeta fetchSchema(Connection connection, String tableName) throws | |||
| String sql = "SELECT * FROM " + ColumnUtils.addEscape(tableName, JdbcConstants.MARIADB) + " LIMIT 1"; | |||
| try (Statement stmt = connection.createStatement(); | |||
| ResultSet rs = stmt.executeQuery(sql)) { | |||
| return resultSetMetaToSchema(rs.getMetaData(), connection.getMetaData()); | |||
| return resultSetMetaToSchema(connection, stmt, rs, rs.getMetaData(), connection.getMetaData()); | |||
There was a problem hiding this comment.
为什么要增加rs.getMetaData(), connection.getMetaData()多个入参，在拥有rs，和connection后，本身就可以再内部获取
Why add multiple parameters for rs.getMetaData() and connection.getMetaData() when we can already obtain them internally once we have rs and connection?
There was a problem hiding this comment.
更直接的体现给想要实现该接口的开发者。并不是所有开发人员都知道，可以从metadata中，获取到rs嘛。降低门槛。
There was a problem hiding this comment.
更直接的体现给想要实现该接口的开发者。并不是所有开发人员都知道，可以从metadata中，获取到rs嘛。降低门槛。
我的意思是在resultSetMetaToSchema方法中单独获取，难道这样换个地方get开发者就看不明白了吗？
What I mean is to obtain it separately in the resultSetMetaToSchema method. Does it mean that developers will not understand it if we get it from a different place?
| @@ -145,7 +156,14 @@ protected TableMeta resultSetMetaToSchema(ResultSetMetaData rsmd, DatabaseMetaDa | |||
| if (tm.getAllColumns().containsKey(col.getColumnName())) { | |||
| throw new NotSupportYetException("Not support the table has the same column name with different case yet"); | |||
| } | |||
|
|
|||
| for (IColumnMetaProcessor processor : processors) { | |||
| processor.process(col, columnIndex, context); | |||
There was a problem hiding this comment.
为什么我们需要多个processor都运行，我认为一个足以？如果有多个，反而可能造成混乱的结果。而且我认为process应该按照当前datasource的dbtype进行加载，否则在用户有多个db数据源下会出现异常
Why do we need multiple processors to run? I think one is sufficient. If there are multiple processors, it might actually lead to confusing results. Moreover, I believe the processes should be loaded based on the current datasource's DB type; otherwise, it may cause exceptions when the user has multiple DB data sources.
| @@ -26,7 +26,7 @@ | |||
| * | |||
| * @author ph3636 | |||
| */ | |||
| @LoadLevel(name = "FastThreadLocalContextCore", order = Integer.MIN_VALUE + 1) | |||
| @LoadLevel(name = "FastThreadLocalContextCore", order = Integer.MIN_VALUE + 1, dependsOnClasses = FastThreadLocal.class) | |||
There was a problem hiding this comment.
为什么要把不想干的代码一起改了？
Why did you change the unrelated code together?
This reverts commit 00e2a16.
Same as #6070 commits into the branch
2.x