none
SELECT COUNTで複数の条件を指定する RRS feed

  • 質問

  • VS2012、VB2012をWin7で利用しています。

    2つのテーブルがあり、稼働時間と稼働した人を管理しています。

    ○Time_Table
    Nippou_ID
    Time_ID
    Start_Time

    End_Time

    ○Member_Table
    Nippou_ID
    Time_ID
    Member_ID



    となっています。

    Time_Tableを元にしたDataGridViewに総稼働時間と人数を追加表示させたいのが最終目標なのですが、まずはTime_Tableの「Nippou_ID」毎に「Time_ID」がいくつあるかを調べ、順にTime_ID毎の人数を「Member_Table」から取得しようとしています。

            Using connection As New SqlClient.SqlConnection(My.Settings.***********ConnectionString)
                Dim SQLTime As SqlCommand = connection.CreateCommand
                Dim SQLMember As SqlCommand = connection.CreateCommand
                Dim TimeAdapter As New SqlDataAdapter(SQLTime)
                Dim MemberAdapter As New SqlDataAdapter(SQLMember)
                Dim TimeTable As New DataTable
                Dim MemberTable As New DataTable
    
                NID = "201312131447"  '←本来は変数が入る
    
                SQLTime.CommandText = "SELECT Nippou_ID , Time_ID , Start_Time, End_Time FROM Time_Table WHERE Nippou_ID = " & "'" & NID & "'"
                TimeAdapter.Fill(TimeTable)
    
                Dim i As Integer    'TimeTableのLoopCounter
    
                For i = 0 To TimeTable.Rows.Count - 1
    
                    Dim x = TimeTable.Rows(i).Item("Time_ID")
    
                    Console.WriteLine("稼働時間 枝" & x)
    
                    SQLMember.CommandText = "Select COUNT(Time_ID) as rec From Member_Table WHERE Nippou_ID = " & "'" & NID & "'" & " AND Time_ID = " & x
    
    
                    MemberAdapter.Fill(MemberTable)
                    Console.WriteLine("MemberのFillは" & MemberTable.Rows(0).Item("rec"))
    
                    MemberTable.Dispose()
                Next
    
                '▼値の表示
                'dgvList.DataSource = Table
    
                '▼後処理
                TimeTable.Dispose()
                TimeAdapter.Dispose()
                SQLTime.Dispose()
                connection.Dispose()
            End Using
    

    しかし、「SQLMember.Commandtxt」のSQL文がおかしいらしく、DBにある正しいレコードをカウント出来ません(全て1件でカウントしてしまっている)。

    条件の所の

    WHERE Nippou_ID = " & "'" & NID & "'" & " AND Time_ID = " & x

    を、

    WHERE Nippou_ID = " & "'" & NID & "'" & " AND Time_ID = 3" 

    と変数ではなく直に数字を入れてやると正しくカウントされます。

    変数xはint型です(データベースのフィールドもint型)。String型の変数NIDはちゃんと抽出出来ているのでこの辺りが原因だとは思うのですが正しい書き方が分かりません。

    ご教授の程よろしくお願い致します。



    2013年12月24日 6:28

回答

  • SqlDataAdapterはFillする度にデータテーブルに追加するという動作をします。よって、何度FillしてもMemberTable.Rows(0)は同じものということになります。よって、Fillする前にデータテーブルをクリアして下さい。

    #どのみち1行しかselectされないのだから、データテーブルを使う必要はないんですけどね。SqlCommand.ExecuteScalarを使えば十分だと思います。


    ★良い回答には回答済みマークを付けよう! わんくま同盟 MVP - Visual C# http://d.hatena.ne.jp/trapemiya/

    • 回答としてマーク chromeMAO 2013年12月24日 9:20
    2013年12月24日 8:51
    モデレータ
  • > 何か他にVS上でやってはいけない表記をしているかと質問した次第です。

    「VS上でやってはいけない」ということではなくて、先にも書きましたように、下記のコードが変です。

    MemberAdapter.Fill(MemberTable)
    Console.WriteLine("MemberのFillは" & MemberTable.Rows(0).Item("rec"))
    MemberTable.Dispose()

    そう言われたのですから、何か変かよく考えてください。でないとレスする甲斐がないです。

    決定的に変なのが MemberTable.Dispose() です。

    たぶん Dispose の代わりに Clear を使ったらうまく行くのではないですか?

    それから、間違いとはいえないかもしれませんが、DataAdapter を使う必要は全くないはず。ExecuteScalar メソッドを使った方がよいです。以下のページのサンプルコードをよく見てください。パラメータ化のヒントもあります。

    SqlCommand.ExecuteScalar メソッド
    http://msdn.microsoft.com/ja-jp/library/system.data.sqlclient.sqlcommand.executescalar(v=vs.110).aspx

    • 回答としてマーク chromeMAO 2013年12月24日 9:20
    2013年12月24日 8:55

すべての返信

  • クエリがパラメータ化してないし、コードもメチャクチャな感じがしますが、とりあえずそれは置いといて・・・

    WHERE Nippou_ID = " & "'" & NID & "'" & " AND Time_ID = " & x

    が NG で、

    WHERE Nippou_ID = " & "'" & NID & "'" & " AND Time_ID = 3"

    が OK ということですから、デバッガを使って、上記それぞれのコードで組み立てられた最終的な文字列(即ち、SQLMember.CommandText に代入された文字列)がどうなっているか調べてみてください。それからヒントが見つかるのではないかと思います。

    デバッガの使い方が分からなければ、ググって調べてください。ブレークポイントの設定、変数の見方など基本的なことだけでいいです。



    • 編集済み SurferOnWww 2013年12月24日 7:19 誤記訂正:OK ←→ NG
    2013年12月24日 7:13
  • Dim x = TimeTable.Rows(i).Item("Time_ID")

    とりあえずこの部分の x の宣言は Dim x as Integer にしましょう。代入値も適切にキャストします。

    そしてデバッガーなどでその変数の使用時の値を確認しましょう。

    また、SQL インジェクションの心配はありませんか?
    必要であれば以下などを参考に SQL のパラメタ化をしましょう。
    http://msdn.microsoft.com/ja-jp/library/yy6y35y8(v=vs.110).aspx

    2013年12月24日 7:13
  • SurferOnWww様、galaco様

    返信ありがとうございます。

    デバッガーでは

    "SELECT COUNT(Time_ID) as rec From Member_Table WHERE Nippou_ID = '201312131447' AND Time_ID = 1"

    がSQLMember.CommandTextに代入されています。For Nextが進むとxも変わる事を確認しました。

    が、抽出されないというのはやはり「SQLのパラメータ化」をしていないのが一番の原因という事でしょうか…?

    2013年12月24日 7:41
  • パラメタ化は本質的な問題ではないでしょうね。

    デバッガーで取得した SQL を management studio などで実行してみて意図した結果を取得できるかどうかなども確認してみてください。

    2013年12月24日 7:56
  • > 抽出されないというのはやはり「SQLのパラメータ化」をしていないのが一番の原因という事でしょうか…?

    クエリが正しければ、抽出されないのは WHERE  句の条件が true にならないか、そうでなければ抽出されたかどうかを判断するために見ているものが違うからだと思います。パラメータ化は関係ないです。

    Nippou_ID = '201312131447' AND Time_ID = 1 というレコードは当該テーブルの中に間違いなくあるのでしょうか?

    具体的にどこをどう見て「抽出されない」という判断をしているのでしょうか?


    #下記のコードが変です。他に書き方があると思いますけど。

    MemberAdapter.Fill(MemberTable)
    Console.WriteLine("MemberのFillは" & MemberTable.Rows(0).Item("rec"))
    MemberTable.Dispose()

    2013年12月24日 8:02
  • galaco様

    度々ありがとうございます。

    SQL Server Management Studioでも確認はしており、再度「新しいクエリ」を開きコピーしたSQL文を貼り付け実行してみた所、やはり正常な結果が得られました。

    後他にやってみたことは、

    「WHERE Nippou_ID = " & "'" & NID & "'" & " AND Group by Time_ID"

    としてグループ化したカウントが取れるか試した所、正常な値が取れました。

    なのでてっきり「WHERE Nippou_ID = " & "'" & NID & "'" & " AND Time_ID = " & x」の

    書き方がまずいだけなのかと思っておりました…

    他に確認するべき所はありますでしょうか。

    2013年12月24日 8:06
  • SurferOnWww様

    返信ありがとうございます。

    >Nippou_ID = '201312131447' AND Time_ID = 1 というレコードは当該テーブルの中に間違いなくあるのでしょうか?

    >具体的にどこをどう見て「抽出されない」という判断をしているのでしょうか?

    挙動が正しいか(レコード自体があるか)はSQL ServerのDBをmanagement studioで確認しながら行っているので、間違いなくテーブルに存在します。具体的に…は

    >「WHERE Nippou_ID = " & "'" & NID & "'" & " AND Time_ID = " & x」

    >が NG で、

    >「WHERE Nippou_ID = " & "'" & NID & "'" & " AND Time_ID = 3"」

    >が OK

    から判断しました。Member_table内にはNippou_ID = '201312131447' AND Time_ID = 1 が1件、Time_ID=2が1件、Time_ID = 3が2件というデータが存在しており、Group By で Countした結果を

    Console.WriteLine("MemberのFillは" & MemberTable.Rows(0).Item("rec"))

    Console.WriteLine("MemberのFillは" & MemberTable.Rows(1).Item("rec"))

    Console.WriteLine("MemberのFillは" & MemberTable.Rows(2).Item("rec"))

    と無理矢理見てみたのですが(変なのはごめんなさいとりあえずおいておいて)

    1

    1

    2

    で表示されることから

    SQLMember.CommandTextに代入されている式は見た目大丈夫そう(SQL Managementで動きもしたのですが)でも何か他にVS上でやってはいけない表記をしているかと質問した次第です。

    2013年12月24日 8:35
  • SqlDataAdapterはFillする度にデータテーブルに追加するという動作をします。よって、何度FillしてもMemberTable.Rows(0)は同じものということになります。よって、Fillする前にデータテーブルをクリアして下さい。

    #どのみち1行しかselectされないのだから、データテーブルを使う必要はないんですけどね。SqlCommand.ExecuteScalarを使えば十分だと思います。


    ★良い回答には回答済みマークを付けよう! わんくま同盟 MVP - Visual C# http://d.hatena.ne.jp/trapemiya/

    • 回答としてマーク chromeMAO 2013年12月24日 9:20
    2013年12月24日 8:51
    モデレータ
  • > 何か他にVS上でやってはいけない表記をしているかと質問した次第です。

    「VS上でやってはいけない」ということではなくて、先にも書きましたように、下記のコードが変です。

    MemberAdapter.Fill(MemberTable)
    Console.WriteLine("MemberのFillは" & MemberTable.Rows(0).Item("rec"))
    MemberTable.Dispose()

    そう言われたのですから、何か変かよく考えてください。でないとレスする甲斐がないです。

    決定的に変なのが MemberTable.Dispose() です。

    たぶん Dispose の代わりに Clear を使ったらうまく行くのではないですか?

    それから、間違いとはいえないかもしれませんが、DataAdapter を使う必要は全くないはず。ExecuteScalar メソッドを使った方がよいです。以下のページのサンプルコードをよく見てください。パラメータ化のヒントもあります。

    SqlCommand.ExecuteScalar メソッド
    http://msdn.microsoft.com/ja-jp/library/system.data.sqlclient.sqlcommand.executescalar(v=vs.110).aspx

    • 回答としてマーク chromeMAO 2013年12月24日 9:20
    2013年12月24日 8:55
  • trapemiya様、SurferOnWww様

    返信ありがとうございます。

    >Fillする前にデータテーブルをクリアして下さい。

    >決定的に変なのが MemberTable.Dispose() です。

    Selectがおかしいと頭から決め込んでそこに集中してしまいました…折角ご助言頂いているのに1度で分からず申し訳ありません。

    仰る通り、Clearで期待する値が取得出来ました。

    本当にありがとうございました。

    御両名から指摘頂いているSqlCommand.ExecuteScalarに関してもこれから実装し直します。

    とても勉強になりました。本当にありがとうございました。

    2013年12月24日 9:20
  • 解決したようですので、参考までに私が気になったところを述べておきます。
    まず、usingを使われているようですが、usingを使った場合、Disposeを実行する必要はありません。Disposeは自動的に実行されます。言い換えれば、Disposeの記述を忘れることなく自動的にDisposeを実行するためのusingと言っても良いでしょう。また、usingは入れ子にできます。以下を参考にしてみて下さい。

    Usingの使い方について
    http://social.msdn.microsoft.com/Forums/ja-JP/520c5921-4e40-445d-8b0e-a4b9b1d4355b/using?forum=netfxgeneralja

    次にSQLですが、以下のSQLだけで目的が達成されませんか?

    select max(Nippou_id) as Nippou_ID, max(Time_ID) as Time_ID, COUNT(*) as 人数
        from dbo.Member_ID
        where Nippou_ID = [実際の値] group by Time_ID

    あと、細かな仕様がわからずに言うのも何ですが、直感的にテーブルの設計がおかしいような気がします。もし、自信がなければ、再度、新しいスレッドを作成して質問して下さい。

    最後にSQLインジェクションについてです。SQLを文字列連結で作成してはいけません。SQLインジェクションにつながるからです。今回の場合は、ユーザーが入力した値を文字列連結しているわけではないのでSQLインジェクションは起きないと思われますが、それでも動的に変わる部分は文字列連結ではなくパラメーターを使う癖を付けられた方が良いでしょう。
    「SQLを文字列連結で作成してはいけません」と先に書きましたが、本当にこの危険性を理解しているのであれば、SQLを文字列連結してもかまいません。大事なことは、なぜSQLインジェクションがおこるのかを理解し、常に意識することです。

    #NIDの値をユーザーが直接入力するのであれば、SQLインジェクションの危険があります。

     

    ★良い回答には回答済みマークを付けよう! わんくま同盟 MVP - Visual C# http://d.hatena.ne.jp/trapemiya/


    2013年12月25日 1:53
    モデレータ
  • trapemiya様

    返信に気付かず、こんなに遅くなってしまって申し訳ありません。

    >Disposeの記述を忘れることなく自動的にDisposeを実行するためのusingと言っても良いでしょう。

    Usingの使い方、とても参考になりました。上記一文はここまでずばりと書いてあるページは探せずにいた為すごく勉強になりました。

    SQLとテーブル設計ですが質問当初は「レコードをカウントしたいのに出来ない」ので「抽出SQLがおかしい」のだと思い、テーブル構成を分かり易く簡単なものに変えてSQLが間違っているか質問してみよう、と思った為実際のテーブル構成とは異なっています。こういった質問の仕方は駄目ですね。反省しています。

    SQLインジェクションについては皆様からご指摘を頂いた後に色々調べ、SQL文によって不正DB操作が出来てしまう危険性を理解しました。現在作っているアプリケーションの殆どは入力値から値を持ってくる仕様ではないのですが、危険性があるなら普段からやっておく習慣をつけた方がいいと思いて現在全部書き換え中です。

    分からない事だらけで心が折れそうですが、こうして返信や参考を頂く事が出来るとがんばって自分の物にしていこうと思う事が出来ます。

    本当にありがとうございました。

    2014年1月23日 0:54