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

What should we using ResultSet directly? #316

Closed
opengl-8080 opened this issue Jun 6, 2019 · 7 comments
Closed

What should we using ResultSet directly? #316

opengl-8080 opened this issue Jun 6, 2019 · 7 comments

Comments

@opengl-8080
Copy link

Doma2 で ResultSet を直接利用したい場合、 @SqlProcessor を使って次のように実装するのが正しいのでしょうか?

Is the following implemenentation correct when using ResultSet directly?

dao.select((config, preparedSql) -> {
    try (
        Connection con = config.getDataSource().getConnection();
        Statement stmt = con.createStatement();
        ResultSet rs = stmt.executeQuery(preparedSql.getFormattedSql());
    ) {
        useResultSet(rs);
        return null;
    } catch (SQLException e) {
        throw new RuntimeException(e);
    }
});

テンプレート処理は Doma2 に任せたいと考えています。
この場合、 SQL は getFormattedSql() で取得したものを使うことで問題ないのかが気になっています(PreparedStatement のプレースホルダを使わないことによるセキュリティリスクがあったりするのか、など)。

I care about whether using the SQL obtaining by getFormattedSql() has any problems.

よろしくお願いします。

@nakamura-to
Copy link
Member

もしSQLテンプレートがバインド変数を持っているとしたら、getFormattedSql() から返されるSQLは安全ではないです。getRawSql()PreparedStatement を使うのを推奨します。

If your SQL template has any bind variables, the SQL returned from getFormattedSql() is unsafe. We recommend to use getRawSql() and PreparedStatement:

dao.select(parameter1, parameter2, (config, preparedSql) -> {
    try (
        Connection con = config.getDataSource().getConnection();
        PreparedStatement stmt = con.prepareStatement(preparedSql.getRawSql());
        // pass parameters via PreparedStatement
        stmt.setString(1, parameter1);
        stmt.setString(2, parameter2);
        ResultSet rs = stmt.executeQuery();
    ) {
        useResultSet(rs);
        return null;
    } catch (SQLException e) {
        throw new RuntimeException(e);
    }
});

@backpaper0
Copy link
Member

少し複雑ですがConfig.getCommandImplementorsをoverrideしてカスタマイズしたSelectCommandを使ってResultSetを処理することも出来ると思います。

SelectCommand.handleResultSetをカスタマイズする感じです。

@opengl-8080
Copy link
Author

@nakamura-to

ご回答ありがとうございます。
やはり、安全ではないんですね。

ちなみに、 PreparedSqlgetParameters() で取得できる SqlParameter は、単純にバインドされる順番に並んでいるわけではないのでしょうか?

できれば、次のような ResultSet を受け取るだけのインターフェースを作れれば、使い勝手が良いかなと思ってたりするのですが。

public interface ResultSetProcessor extends BiFunction<Config, PrepraredSql, Void> {
    void process(ResultSet resultSet) throws SQLException;

    @Override
    default Void apply(Config config, PreparedSql sql) {
        try (
            Connection con = config.getDataSource().getConnection();
            ...
            ResultSet resultSet = ps.executeQuery();
        ) {
            this.process(resultSet);
            return null;
        } catch (SQLException e) {
            throw new RuntimeException(e);
        }
    }
}

...

dao.select(parameter1, parameter2, resultSet -> {
    useResultSet(resultSet);
});

@backpaper0

ResultSetHandler が何をしているのかと、 return している Supplier<RESULT> がどういうものになるべきなのかがちょっと理解できていないので、カスタマイズはやや抵抗がありますねー。

@nakamura-to
Copy link
Member

ちなみに、 PreparedSql の getParameters() で取得できる SqlParameter は、単純にバインドされる順番に並んでいるわけではないのでしょうか?

並んでいます。

できれば、次のような ResultSet を受け取るだけのインターフェースを作れれば、使い勝手が良いかなと思ってたりするのですが。

なるほど。あくまで ResultSet に直接触れれば良いのですね。 @SqlProcessor はテンプレート処理された後のSQLをさらに手を入れたかったり、 PreparedStatement がつかえずリスク承知で Statement を使うしかなかったりした場合を想定した機能なので、ちょっとミスマッチかもしれませんね。

ちなみに、 ResultSet に直接触りたい理由はどんなものでしょうか?理由によっては別の方法を提案できるかもしれません。

@opengl-8080
Copy link
Author

並んでいます。

あら、そうでしたか。
ちょっと試したらうまく動作しなかったので違うのかなと思ってました。
勘違いしてるっぽいので、もうちょっと詳しく確認してみます。

ちなみに、 ResultSet に直接触りたい理由はどんなものでしょうか?

ResultSet を直接使用したい理由は、 JasperReports という帳票ライブラリの入力に使用したいためです。(JRResultSetDataSource

JRXML に SQL を組み込めることは知っているのですが、 SQL テンプレートの書きやすさやメンテのしやすさから、検索までは Doma2 にしてもらって、 ResultSet だけ欲しいなぁという思いです。

@nakamura-to
Copy link
Member

ちょっと試したらうまく動作しなかったので違うのかなと思ってました。

並び順はバインド順なんですが、データ構造がDomaに都合のいい形なので PreparedStatement にバインドできるデータを取り出すのに工夫が必要かもしれません。

ResultSet を直接使用したい理由は、 JasperReports という帳票ライブラリの入力に使用したいためです。

なるほど。JasperReports使ったことないので推測で書きますが、 JRDataSourceを見るといろんな実装があるようなので、ResultSetではなくJavaBeansとかMapを渡す方法を検討してもいいかもしれませんね。JavaBeansやMapであれば @SqlProcessor を使わないで対応できそうです。(通常の @Select で対応できそうです)

@opengl-8080
Copy link
Author

JavaBeansとかMapを渡す方法を検討してもいいかもしれませんね。

JavaBeans より ResultSet のほうがマッピングの処理がない分、効率的だと勝手に思いこんでました。
今日ためしにメモリ消費や処理時間を確認したところ、どちらもほとんど変わらない結果になりました。

なので、おっしゃられている通り JavaBeans の List を返す通常の @Select で対応しようと思います。
(issue も close させていただきます)

お騒がせしました。
ありがとうございました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants