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

Added oracle database support #902

Merged
merged 68 commits into from Jul 31, 2019
Merged

Conversation

15168326318
Copy link
Contributor

@15168326318 15168326318 commented Apr 25, 2019

重新提一次pr,和#649 的一样。

@lovepoem
Copy link
Member

Now the com/alibaba/fescar is changed to io/seata

@lovepoem lovepoem added this to the 0.8.0 milestone Apr 25, 2019
@xingfudeshi
Copy link
Member

thanks.could you do us a favor?the package name and copyright need to be updated.

@xingfudeshi xingfudeshi added the type: feature Category issues or prs related to feature request. label Apr 25, 2019
pom.xml Outdated
<repository>
<id>openSource</id>
<url>https://oss.sonatype.org/service/local/staging/deploy/maven2/</url>
<id>releases</id>
Copy link
Member

Choose a reason for hiding this comment

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

why changed?

Copy link
Member

Choose a reason for hiding this comment

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

Please revert the following:

<distributionManagement>
                <snapshotRepository>
                    <id>oss_seata</id>
                    <url>https://oss.sonatype.org/content/repositories/snapshots</url>
                </snapshotRepository>

15168326318 and others added 3 commits April 25, 2019 14:50
…lop_seata_oracle

# Conflicts:
#	pom.xml
#	rm-datasource/src/main/java/io/seata/rm/datasource/ConnectionProxy.java
#	rm-datasource/src/main/java/io/seata/rm/datasource/DataSourceManager.java
#	rm-datasource/src/main/java/io/seata/rm/datasource/exec/BaseTransactionalExecutor.java
#	rm-datasource/src/main/java/io/seata/rm/datasource/sql/SQLVisitorFactory.java
#	rm-datasource/src/main/java/io/seata/rm/datasource/undo/KeywordCheckerFactory.java
#	rm-datasource/src/main/java/io/seata/rm/datasource/undo/UndoExecutorFactory.java
#	server/src/main/resources/registry.conf
@@ -36,6 +36,8 @@
public static KeywordChecker getKeywordChecker(String dbType) {
if (dbType.equals(JdbcConstants.MYSQL)) {
return MySQLKeywordChecker.getInstance();
} else if (dbType.equals(JdbcConstants.ORACLE)) {

Choose a reason for hiding this comment

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

Maybe using switch here makes things more readable - I guess there will be even more types in the future.

/**
* The type Table meta cache.
*/
public class TableMetaCacheOracle {

Choose a reason for hiding this comment

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

This class is only used by the new undo manager, but the BaseTransactionalExecutor still uses the default TableMetaCache.
Additionally the resultSetMetaToSchema method already contains parts for "H2 drivers" and other stuff. So I would think of having additional Oracle stuff in the already existing TableMetaCache instead of inventing another class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我觉得还是在BaseTransactionalExecutor 这里加比较好。

Choose a reason for hiding this comment

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

Ok, but why does this class then contain some "H2 driver" parts? I mean this class is for Oracle only, right?
(see line 281)

@@ -66,7 +66,19 @@ public static SQLRecognizer get(String sql, String dbType) {
recognizer = new MySQLSelectForUpdateRecognizer(sql, ast);
}
}
} else {
} else if (JdbcConstants.ORACLE.equalsIgnoreCase(dbType)) {

Choose a reason for hiding this comment

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

Not sure if equalsIgnoreCase is really required here. Otherwise a switch statement would be a better solution.

@@ -184,7 +185,11 @@ private void processGlobalTransactionCommit() throws SQLException {

try {
if (context.hasUndoLog()) {
UndoLogManager.flushUndoLogs(this);
if(JdbcConstants.ORACLE.equalsIgnoreCase(this.getDbType())) {

Choose a reason for hiding this comment

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

Maybe some factory class should take care of returning an undo manager. Otherwise you will have this kind of logic at several places.

Copy link
Member

Choose a reason for hiding this comment

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

io.seata.rm.datasource.undo.*; is a wrong type

@@ -184,7 +185,11 @@ private void processGlobalTransactionCommit() throws SQLException {

try {
if (context.hasUndoLog()) {
UndoLogManager.flushUndoLogs(this);
if(JdbcConstants.ORACLE.equalsIgnoreCase(this.getDbType())) {
Copy link
Member

Choose a reason for hiding this comment

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

io.seata.rm.datasource.undo.*; is a wrong type

import io.seata.rm.datasource.sql.struct.TableMeta;
import io.seata.rm.datasource.sql.struct.TableMetaCache;
import io.seata.rm.datasource.sql.struct.TableRecords;
import io.seata.rm.datasource.sql.struct.*;
Copy link
Member

Choose a reason for hiding this comment

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

  • is a wrong type

pom.xml Outdated
<repository>
<id>openSource</id>
<url>https://oss.sonatype.org/service/local/staging/deploy/maven2/</url>
<id>releases</id>
Copy link
Member

Choose a reason for hiding this comment

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

Please revert the following:

<distributionManagement>
                <snapshotRepository>
                    <id>oss_seata</id>
                    <url>https://oss.sonatype.org/content/repositories/snapshots</url>
                </snapshotRepository>

@codecov-io
Copy link

codecov-io commented Apr 26, 2019

Codecov Report

Merging #902 into develop will decrease coverage by 2.56%.
The diff coverage is 0.59%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #902      +/-   ##
=============================================
- Coverage      48.32%   45.76%   -2.57%     
  Complexity      1653     1653              
=============================================
  Files            333      343      +10     
  Lines          11669    12322     +653     
  Branches        1448     1549     +101     
=============================================
  Hits            5639     5639              
- Misses          5401     6053     +652     
- Partials         629      630       +1
Impacted Files Coverage Δ Complexity Δ
...a/io/seata/rm/datasource/exec/ExecuteTemplate.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...tasource/undo/oracle/OracleUndoInsertExecutor.java 0% <0%> (ø) 0 <0> (?)
.../rm/datasource/exec/BaseTransactionalExecutor.java 33.89% <0%> (-1.82%) 6 <0> (ø)
.../io/seata/rm/datasource/sql/SQLVisitorFactory.java 21.42% <0%> (-11.91%) 2 <0> (ø)
...tasource/undo/oracle/OracleUndoDeleteExecutor.java 0% <0%> (ø) 0 <0> (?)
...eata/rm/datasource/undo/KeywordCheckerFactory.java 16.66% <0%> (-8.34%) 1 <0> (ø)
...seata/rm/datasource/undo/UndoLogManagerOracle.java 0% <0%> (ø) 0 <0> (?)
...java/io/seata/rm/datasource/DataSourceManager.java 25.75% <0%> (-1.67%) 7 <0> (ø)
...n/java/io/seata/rm/datasource/ConnectionProxy.java 9.09% <0%> (-0.22%) 4 <0> (ø)
...ource/sql/druid/oracle/OracleUpdateRecognizer.java 0% <0%> (ø) 0 <0> (?)
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21add28...6f58367. Read the comment docs.

@15168326318
Copy link
Contributor Author

BaseTransactionalExecutor冲突解决了

@@ -28,7 +28,7 @@ transport {
}
service {
#vgroup->rgroup
vgroup_mapping.my_test_tx_group = "default"
vgroup_mapping.txServiceGroup = "default"
Copy link
Member

Choose a reason for hiding this comment

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

revert

README.md Outdated Show resolved Hide resolved
@slievrly
Copy link
Member

image

@slievrly
Copy link
Member

缺少context字段
image

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

For metadata problems that must use the uppercase table name to get the oracle database, it is recommended to convert directly to uppercase.

@15168326318
Copy link
Contributor Author

bug都修改完成了。 @slievrly

@@ -76,7 +76,6 @@ store {
datasource = "dbcp"
## mysql/oracle/h2/oceanbase etc.
db-type = "mysql"
driver-class-name = "com.mysql.jdbc.Driver"
Copy link
Member

Choose a reason for hiding this comment

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

revert

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature Category issues or prs related to feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants