SQLファイルを使った@Updateメソッドの実行時、実行がスキップされることがある。

[MakotoKinoshita]

domaの1.26.0から2.19.1にアップデートする作業をしております。
バージョン2.19.1において、SQLファイルを実行する一部のUpdateメソッドを実行した際、以前は実行されたSQLの実行がスキップされるようになりました。

事象

SQLファイルを実行するUpdateメソッドを実行した結果、処理がスキップされてレコードが更新されなかった。1.26.0時点では、SQLを使った更新は、必ずSQLが実行される想定であった。

確認事項

ソースを確認すると、#75 でSqlFileUpdateQuery等に修正が入っており、更新対象のプロパティのみが更新された場合にUpdateを実行するように変更されているようにみえます。
※/*%populate */は使わないケースでも事象が発生します。

そのため、本来であればSQLが実行されることを期待するケースで実行されない事があると思われます。
ご確認の程、よろしくお願いいたします。

問題が発生する可能性のあるケース

• エンティティのプロパティは変更しないが、SQLファイル内で、例えばOracleのSYSDATE等を使ってレコードを更新したい場合
• 同じエンティティで、SQLを使わないUPDATEと、使うUPDATEのDaoメソッドを混在させて、使い分けしたい場合

コード抜粋

※一部マスキングしております

    @Update(sqlFile = true)
    int updateXXX(XXXBean arg);

public class XXXBean extends AbstractEntity<XXXKey, XXX> implements XXX {

    /** OriginalStates */
    @SuppressWarnings("unused")
    @OriginalStates
    private XXXBean originalStates;
・・・
    /** 更新日時 */
    @Column(insertable = false, updatable = false)
    private YyyyMmDdHhMmSs updDatetime; // ★日時を変更してSQL実行
・・・

UPDATE 
    XXXXXX xxxx
SET
    xxxx.updUserInternalNo = /* arg.updUserInternalNo */'0000001'
    ,xxxx.updSysDatetime = /* arg.updSysDatetime */'200701010101002'
    ,xxxx.updBizDate = /* arg.updBizDate */'20070101'
WHERE
    xxxx.tranDataInternalNo = /* arg.tranDataInternalNo */0

[nakamura-to]

原因わかりました。ここのコードですね。

https://github.com/domaframework/doma/blob/2.19.1/src/main/java/org/seasar/doma/jdbc/query/UpdateQueryHelper.java#L94

#75 の修正で共通化し過ぎてしまったようです。
今週末には修正してリリースしようと思います。

[nakamura-to]

すみません。修正を検討していましたが、1.26.0と挙動が変わっているとは言え妥当性があるので現行の仕様を維持したいと考えています。

ドキュメントには反映されていませんが、現行の仕様は次のようになっています。

• @OriginalStatesが注釈されたエンティティが変更されていなかったらSQLを実行しない
• 変更チェックは@Update(includeUnchanged = true)で無効にできる

アプリケーション側での対応策ですが、以下のようにしてもらうのはどうでしょうか？

@Update(sqlFile = true, includeUnchanged = true)
int updateXXX(XXXBean arg);

@Update(includeUnchanged = true)とすることでスキップされることなく必ずSQLが実行されます。

[makotok]

回答ありがとうございます。
対応を検討してみます。
※個人的な都合ですが、修正対象が1000個以上あるのでどうしようかなと思っています。

[makotok]

Javadocを確認しました。

doma/src/main/java/org/seasar/doma/Update.java

Lines 105 to 115 in 319828b

	/**
	* UPDATE文のSET句に変更されていないプロパティに対応するカラムを含めるかどうかを返します。
	* <p>
	* この要素に対する指定は、更新対象のエンティティが {@link OriginalStates} が注釈されたプロパティをもつ場合、かつ
	* {@link #sqlFile()} が {@code false} の場合にのみ有効です。
	* <p>
	* この要素に対する指定は、{@link #sqlFile()} が {@code false} の場合にのみ有効です。
	*
	* @return カラムを含めるかどうか
	*/
	boolean includeUnchanged() default false;

includeUnchangedは、Javadocで確認する限り、sqlFileがfalseの時だけ有効である仕様のようです。
念のための確認ですが、SQL利用時にこのプロパティを使っても問題ないのでしょうか？

理想的には、以下の仕様の方が動きが分かりやすくて、バグを埋め込まないと思いますが、いかがでしょうか？

• sqlFile=false
  • includeUnchanged=false：更新対象プロパティを変更した場合にSQL実行(SET句は変更項目のみ)
  • includeUnchanged=true：常にSQLを実行(SET句は全項目)
• sqlFile=true,かつ,SET句にpopulate利用時
  • includeUnchanged=false：更新対象プロパティを変更した場合にSQL実行(SET句は変更項目のみ)
  • includeUnchanged=true：常にSQLを実行(SET句は全項目)
• sqlFile=true,かつ,SET句にpopulate利用せずに、普通に項目を指定
  • includeUnchanged=false：常にSQLを実行(SET句は指定項目)
  • includeUnchanged=true：常にSQLを実行(SET句は指定項目)

[nakamura-to]

今回はJavaDocコメントが正しくないと判断しました。JavaDocコメントは次のリリースでは修正しようと思います。提案いただいたpopulateの有り無しで挙動を変える方法は検討してみたのですが、少し複雑だと（シンプルさが足りないと）感じました。

※個人的な都合ですが、修正対象が1000個以上あるのでどうしようかなと思っています。

これは大変ですね。アプリケーションで次のように対応すると楽だと思います。

QueryImplementorsを実装したクラスを用意する

import java.lang.reflect.Method;

import org.seasar.doma.jdbc.QueryImplementors;
import org.seasar.doma.jdbc.query.SqlFileUpdateQuery;

public class MyQueryImplementors implements QueryImplementors {
    @Override
    public SqlFileUpdateQuery createSqlFileUpdateQuery(Method method) {
        SqlFileUpdateQuery query = new SqlFileUpdateQuery();
        query.setUnchangedPropertyIncluded(true); // ここがポイント
        return query;
    }
}

Configの実装クラスで上記クラスのインスタンスを返す

public class AppConfig implements Config {
    @Override
    public QueryImplementors getQueryImplementors() {
        return new MyQueryImplementors();
    }

   ...

}

こうすることで、既存の @Update(sqlFile = true) を @Update(sqlFile = true, includeUnchanged = true) と書き直すのと同じ効果があります。

[MakotoKinoshita]

回答ありがとうございます！
その実装方法であれば対応できそうなので、試してみます。

[MakotoKinoshita]

ご教示頂いた実装方法だと、自動性生成されたDapImplによって、unchangedPropertyIncludedが上書きされてしまうようです。上書きされないように、setUnchangedPropertyIncludedを無効化するか、isExecutable()が必ずtrueで返すように対応してみたいと思います。

__query.setUnchangedPropertyIncluded(false);

追記：最終的にこのように実装しました。

public class MyQueryImplementors implements QueryImplementors {

    private static class MySqlFileUpdateQuery extends SqlFileUpdateQuery {

        public MySqlFileUpdateQuery() {
            super();
            unchangedPropertyIncluded = true;
        }

        @Override
        protected void prepareExecutable() {
            executable = true;
            sqlExecutionSkipCause = null;
        }

        @Override
        public void setUnchangedPropertyIncluded(Boolean unchangedPropertyIncluded) {
            // unchangedPropertyIncludedプロパティの設定変更は無効化
        }
    }

    public StrQueryImplementors() {
    }

    @Override
    public SqlFileUpdateQuery createSqlFileUpdateQuery(Method method) {
        return new MySqlFileUpdateQuery();
    }
}

[nakamura-to]

自動性生成されたDapImplによって、unchangedPropertyIncludedが上書きされてしまうようです

おっと、そうでした 😅

追記：最終的にこのように実装しました。

👍

[nakamura-to]

JavaDocコメントは次のリリースでは修正しようと思います。

#223 で修正しました。