bugfix: fix the failure of rollbacking back of the delete SQL at AT m… #6511
Conversation
…ode when using SQLServer (apache#6511)
TakeActionNow2019
force-pushed
the
hotfix-sqlserver-undo-identity-error
branch
from
70b68db
to
1453629
Compare
…ode when using SQLServer (apache#6511)
TakeActionNow2019
force-pushed
the
hotfix-sqlserver-undo-identity-error
branch
from
1453629
to
7104fb0
Compare
|
At AT mode when using SQLServer, if any column of a table is with "IDENTITY", before inserting a value into this column, we should judge whether there is a "IDENTITY" with it or not. Or the database will report errors, ie "Table 'xxx' does not have the identity property. Cannot perform SET operation" and "Cannot insert explicit value for identity column in table 'xxx' when IDENTITY_INSERT is set to OFF." |
| import java.sql.Connection; | ||
| import java.sql.PreparedStatement; | ||
| import java.sql.ResultSet; | ||
| import java.sql.SQLException; | ||
| import java.sql.Statement; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; |
There was a problem hiding this comment.
Please put the Java package at the top level.
|
|
||
| public class SqlServerUndoDeleteExecutor extends BaseSqlServerUndoExecutor { | ||
|
|
||
| private boolean tableIdentifyExistence = false; |
There was a problem hiding this comment.
I hope this variable will eventually be cached in TableMeta and the caching-related refresh and retrieval will be implemented in SqlServerTableMetaCache, rather than in the executor, where it needs to be executed every time executeOn is called.
There was a problem hiding this comment.
Originally, I considered putting it in a cache. But then I thought of that the application has to be started again to refresh the cache when a column that has something to do with ""INDENTITY changed. So I quit to do that.
Now I rethink that it's probable for a application to restart when a column is changed.
I will fix it tonight.
There was a problem hiding this comment.
Done!
I use the SqlServerTableMetaCache to help me. Please help me reivew it again!
There was a problem hiding this comment.
My idea is to have tableIdentifyExistence handled together with resultSetMetaToSchema in SqlServerTableMetaCache, rather than handling it separately in SqlServerUndoDeleteExecutor.
There was a problem hiding this comment.
I pondered about it last night. And I found that the cahce key should be a combination of resourceId, schema and table name. It may be a feasible way to put it in the subclass of TableMeta. But when I was creating a subclass of TableMeta named SqlServerTableMeta, I found it a little elegant again. So I make a compromise and fix it in this way. What do you think of it? Any better ideas？
There was a problem hiding this comment.
There is a typo there. It should be " I found it a little not elegant again". o(╯□╰)o
There was a problem hiding this comment.
I found it a good way to judge whether there is a "IDENTITY" in a table by creating a subclass of TableMeta named SqlServerTableMeta with a new filed in it for justification. But I found there are no other subclass of TableMeta for other SQL dialect. Maybe it sounds a bit inelegant, too. What do you think of it? @funky-eyes
There was a problem hiding this comment.
I found it a good way to judge whether there is a "IDENTITY" in a table by creating a subclass of TableMeta named SqlServerTableMeta with a new filed in it for justification. But I found there are no other subclass of TableMeta for other SQL dialect. Maybe it sounds a bit inelegant, too. What do you think of it? @funky-eyes
I think it's possible to create a subclass to provide specific metadata for different databases. I agree with your approach, let's go ahead and do it.
…ode when using SQLServer (apache#6511)
TakeActionNow2019
force-pushed
the
hotfix-sqlserver-undo-identity-error
branch
from
7104fb0
to
7643244
Compare
funky-eyes
added
type: bug
Ⅰ. Describe what this PR did
fix the failure of rollbacking back of the delete SQL at AT mode when using SQLServer
Ⅱ. Does this pull request fix one issue?
fix #6449
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews