Shardingsphere: when using EncryptPreparedStatement to savebatch occours error

[Keanhua]

Bug Report

For English only, other languages will not accept.

Before report a bug, make sure you have:

• Searched open and closed GitHub issues.
• Read documentation: ShardingSphere Doc.

Please pay attention on issues you submitted, because we maybe need more details.
If no response more than 7 days and we cannot reproduce it on current information, we will close it.

Please answer these questions before submitting your issue. Thanks!

Which version of ShardingSphere did you use?

4.0.0-RC2

Which project did you use? Sharding-JDBC or Sharding-Proxy?

Sharding-JDBC

Expected behavior

preparedStatement is not null

Actual behavior

preparedStatement is null

Reason analyze (If you can)

when use EncryptPreparedStatement to batch operation, preparedStatement will not Initialization

Steps to reproduce the behavior, such as: SQL to execute, sharding rule configuration, when exception occur etc.

Example codes for reproduce this issue (such as a github link).

[betterjava]

@Keanhua can you show you code how to use EncryptPreparedStatement? Because I use the EncryptPreparedStatement and it works well, here is my code:

[tristaZero]

@Keanhua

when use EncryptPreparedStatement to batch operation...
Please show us the SQL and usage for EncryptPreparedStatement.

[Keanhua]

@betterjava @tristaZero

this is mycode by use the mybatis plus。
when i insert batch it occurred the NullPointerException。

[xiyelife]

I have the same problem，only use encryptDataSource by mybatis.
when mybatis prepare the statement, it will set the queryTimeout ,but the preparedStatement does not created at this time, and the NPE occured!

AbstractStatementAdapter:

EncryptPreparedStatement:

here is my config:

user_info.name 加密

spring.shardingsphere.encrypt.encryptors.name_encryptor.type=aes
spring.shardingsphere.encrypt.tables.user_info.columns.name.plainColumn=name
spring.shardingsphere.encrypt.tables.user_info.columns.name.cipherColumn=name_encrypt
spring.shardingsphere.encrypt.tables.user_info.columns.name.encryptor=name_encryptor

开启使用密文列：读取对应的密文字段解密自动放到明文字段上

spring.shardingsphere.props.query.with.cipher.comlum=true
spring.shardingsphere.props.sql.show=true

mybatis-version:
org.mybatis.spring.boot:mybatis-spring-boot-starter:1.3.2
springboot-version:
1.5.5

[tristaZero]

Hi, can you show us the content of settleCompanyService.saveBatch(). What's the action or sql of this function? @xiyelife

[xiyelife]

not only batch insert，the sigle insert occured too ，here is my sql:

encrypted columns: name,ident,mobile

t.mybatis.mapper.UserInfoMapper.insert : ==> Preparing: insert into user_info (cust_id, unique_id, name, ident, mobile, password, reg_time, modify_time, reg_source, reg_app_channel, reg_device_info, status, is_mobile_authed, complate_cust_info, time_inst, time_upd, has_checked) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, now() , ?, ?)

ShardingSphere-SQL : SQL: insert into user_info (cust_id, unique_id, name_encrypt,
ident_encrypt, mobile_encrypt, password,
reg_time, modify_time, reg_source,
reg_app_channel, reg_device_info, status,
is_mobile_authed, complate_cust_info, time_inst,
time_upd, has_checked, mobile, ident, name)
values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, now(), ?, ?, ?, ?, ?)

code is:
UserInfo record = new UserInfo();
record.setMobile(mobile);
record.setName(TestLocalKmssdk.createName());
record.setPassword(oldPass);
int num = new Double(Math.random() * 1000000).intValue();
record.setIdent(ident + num);
record.setRegDeviceInfo(oldDev);
record.setModifyTime("111111");
record.setTimeInst(new Date());
record.setTimeUpd(new Date());
record.setRegSource("111");
record.setIsMobileAuthed("1");
userInfoMapper.insert(record);

@tristaZero

[tristaZero]

@xiyelife Thanks for your particular description. So great, i guess i get the point!

[tristaZero]

@xiyelife Thanks for your kind pr as well. Moreover, can you make this pr become better?
Actually, the solution in this pr can just stop NPE, but it is necessary to sovle it completely, so we should change our mind to stop NPE at beginning.

I suggest we can create a preparedStatement upon EncryptPreparedStatement is created, i.e, we should move preparedStatement = preparedStatementGenerator.createPreparedStatement(sqlUnit.getSql()); from execute() or executeUpdate() to all the constructors of EncryptPreparedStatement.

Therefore once EncryptPreparedStatement is created, the preparedStatement will be created as well, then NPE will never happen.

If you have other advices, please contact me, or maybe you can modify your kind pr to perfect it. 😉

Here are the related codes

public EncryptPreparedStatement(final EncryptConnection connection, final String sql) {
        this.sql = sql;
        preparedStatementGenerator = new EncryptPreparedStatementGenerator(connection);
    }
---
@Override
public int executeUpdate() throws SQLException {
        try {
            SQLUnit sqlUnit = getSQLUnit(sql);
            preparedStatement = preparedStatementGenerator.createPreparedStatement(sqlUnit.getSql());
            replaySetParameter(preparedStatement, sqlUnit.getParameters());
            return preparedStatement.executeUpdate();
        } finally {
            clearParameters();
        }
    }

[xiyelife]

@tristaZero ok ，that's what I thought at first. I'll try to modify my pr later.

And I found that the latest version of the code（RC3-SNAPSHOT） would incorrectly insert my original column like this:
SQLRewriteEngine Pre：
columnNames=[name, ident, is_mobile_authed, mobile, modify_time, password, reg_device_info, reg_source, time_inst, time_upd, mobile, ident, name]
parameters=[闵姐, 600xxx756, 1, 1xxx4, 111111, jpaxsdsewrwe123123f, jpatest, 111, 2019-09-05 15:27:47.77, 2019-09-05 15:27:47.77])]

name=闵姐

mobile=1xxx4

atfter SQLRewriteEngine:
SQLUnit(sql=insert into user_info (name_encrypt, ident_encrypt, is_mobile_authed, mobile_encrypt, modify_time, password, reg_device_info, reg_source, time_inst, time_upd, mobile, ident, name) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?), parameters=[*n/w+WpRwyeiTYgpM59dyF9g==, *i4sLpmagWmoLIiHZ5h7bt1kTeFpBZrxX8jgy/M8y0fU0=, 1, *mN6+s/ObMX0ZTcWp7pl9T6g==, 111111, jpaxsdsewrwe123123f, jpatest, 111, 2019-09-05 15:27:47.77, 2019-09-05 15:27:47.77, 闵姐, 600xxx756, 1xxx4])

name=1xxx4

mobile=闵姐

the two columns are interchanged!!
The data stored in the database are also interchanged.

[tristaZero]

The problem of interchanged column was solved at #2975 yesterday. Thanks, and wait for your pr. 😄

[tristaZero]

@xiyelife Hi, after my investigation, the similar problem also happens in ShardingPreparedStatement. If you're interested in it, can u consider this issue. Don't worry, it does not have deadline.

[xiyelife]

@tristaZero Hi, there are no incoming parameters to bind at the time of construction, which can cause the generation of EncryptInsertOptimizedStatement failed:

I think the problem should be more complicated......

[tristaZero]

Got it, maybe we can split this task to two small ones.
First, if preparedStatemnt == null in getRoutedStatements(), emptyList will be returned. This step will stop NPE, but set queryTime() actually is not executed, for there is no preparedStatement.
Second, we may cache the value of queryTime() in AbstractStatementAdapter.java. When executeUpdate() or execute() is called, at this time, we relay the action, i.e, setQueryTime().

What's your opinion? we can do first one, i think. @xiyelife

[xiyelife]

yeah, I think so, we can deal with the remaining parameters by your way, i.e maxRows,fetchSize and so on. @tristaZero

[tristaZero]

@xiyelife Hi, if first one is done, the second one, i think, there is a easier way to do.

As we said before, it is needed to cache the value of setQueryTime() and when call execute(), this operation will be really performed.

Currently, setQueryTime() in AbstractStatementAdapter.java has cached the value, here it is,

@SuppressWarnings("unchecked")
    @Override
    public final void setQueryTimeout(final int seconds) throws SQLException {
       // cache the value
        recordMethodInvocation(targetClass, "setQueryTimeout", new Class[] {int.class}, new Object[] {seconds});
        forceExecuteTemplate.execute((Collection) getRoutedStatements(), new ForceExecuteCallback<Statement>() {

            @Override
            public void execute(final Statement statement) throws SQLException {
                statement.setQueryTimeout(seconds);
            }
        });
    }

So, we just call replayMethodsInvocation() in EncryptPreparedStatement when call execute() or executeUpdate().

That's all, how about you?

[xiyelife]

@tristaZero yeah, it would be better, and it would solve the ShardingPreparedStatement problem.
sorry，I was busy yesterday. I'm going to change my pr.

[tristaZero]

@xiyelife Hi, Busy though you are, yet you raize this pr. Thanks a lot.
In addition, The problem of ShardingPreparedStatement was solved by @sunbufu Here. So take it easy. 😃