|
dev
newsgroups
|
|||||||||||||||||||||||
|
|||||||||||||||||||||||
Critique My SQL// Query article data SqlDataAdapter adap = new SqlDataAdapter("Select art_name, art_date, art_desc, auth_name," + " '" + ResolveUrl("~/") + "Articles/' + grp_folder + '/' + cat_folder + '/' + art_filename + '.aspx' AS art_path" + " FROM Articles, Authors, Categories, Groups" + " WHERE cat_id = '" + id.ToString() + "' AND art_auth_id = auth_id AND art_cat_id = cat_id AND cat_grp_id = grp_id" + " ORDER BY art_name", conn); DataSet ds = new DataSet(); adap.Fill(ds); // Assign datasource to grid GridView1.DataSource = ds; GridView1.DataBind(); It's working correctly but I'm new to this and have a couple of questions. 1. Does my SQL statement seem reasonably efficient and correctly formed? Or am I asking for performance problems with a page that renders with complex queries? 2. How can I find out if no rows were returned? Thanks! You need to put that query in a stored procedure. You are opening yourself
up to SQL Injection type hacking. You can check ds.tables(0).Rows.Count to see if you got any results. Robin S. ---------------------------- Show quote "Jonathan Wood" <jw***@softcircuits.com> wrote in message news:uvDFhRVUHHA.4828@TK2MSFTNGP05.phx.gbl... > I'm writing a Website that contains the following code: > > // Query article data > SqlDataAdapter adap = new SqlDataAdapter("Select art_name, art_date, > art_desc, auth_name," + > " '" + ResolveUrl("~/") + "Articles/' + grp_folder + '/' + cat_folder + > '/' + art_filename + '.aspx' AS art_path" + > " FROM Articles, Authors, Categories, Groups" + > " WHERE cat_id = '" + id.ToString() + "' AND art_auth_id = auth_id AND > art_cat_id = cat_id AND cat_grp_id = grp_id" + > " ORDER BY art_name", conn); > DataSet ds = new DataSet(); > adap.Fill(ds); > > // Assign datasource to grid > GridView1.DataSource = ds; > GridView1.DataBind(); > > It's working correctly but I'm new to this and have a couple of > questions. > > 1. Does my SQL statement seem reasonably efficient and correctly formed? > Or am I asking for performance problems with a page that renders with > complex queries? > > 2. How can I find out if no rows were returned? > > Thanks! > > -- > Jonathan Wood > SoftCircuits Programming > http://www.softcircuits.com > > "RobinS" <RobinS@NoSpam.yah.none> wrote in message Please don't top post.news:LoednUida7theknYnZ2dnUVZ_qisnZ2d@comcast.com... > You need to put that query in a stored procedure. You are opening yourself > up to SQL Injection type hacking. > > You can check ds.tables(0).Rows.Count to see if you got any results. > > Robin S. > ---------------------------- You could just use a parameterized query to gain performance and avoid SQL injection. Thanks. Putting it in a stored procedure was on my to-do list.
Show quote "RobinS" <RobinS@NoSpam.yah.none> wrote in message news:LoednUida7theknYnZ2dnUVZ_qisnZ2d@comcast.com... > You need to put that query in a stored procedure. You are opening yourself > up to SQL Injection type hacking. > > You can check ds.tables(0).Rows.Count to see if you got any results. > > Robin S. > ---------------------------- > > "Jonathan Wood" <jw***@softcircuits.com> wrote in message > news:uvDFhRVUHHA.4828@TK2MSFTNGP05.phx.gbl... >> I'm writing a Website that contains the following code: >> >> // Query article data >> SqlDataAdapter adap = new SqlDataAdapter("Select art_name, art_date, >> art_desc, auth_name," + >> " '" + ResolveUrl("~/") + "Articles/' + grp_folder + '/' + cat_folder + >> '/' + art_filename + '.aspx' AS art_path" + >> " FROM Articles, Authors, Categories, Groups" + >> " WHERE cat_id = '" + id.ToString() + "' AND art_auth_id = auth_id AND >> art_cat_id = cat_id AND cat_grp_id = grp_id" + >> " ORDER BY art_name", conn); >> DataSet ds = new DataSet(); >> adap.Fill(ds); >> >> // Assign datasource to grid >> GridView1.DataSource = ds; >> GridView1.DataBind(); >> >> It's working correctly but I'm new to this and have a couple of >> questions. >> >> 1. Does my SQL statement seem reasonably efficient and correctly formed? >> Or am I asking for performance problems with a page that renders with >> complex queries? >> >> 2. How can I find out if no rows were returned? >> >> Thanks! >> >> -- >> Jonathan Wood >> SoftCircuits Programming >> http://www.softcircuits.com >> >> > >
Show quote
"Jonathan Wood" <jw***@softcircuits.com> wrote in message No. Or rather, who can tell when you splice together C# and SQL like that. news:uvDFhRVUHHA.4828@TK2MSFTNGP05.phx.gbl... > I'm writing a Website that contains the following code: > > // Query article data > SqlDataAdapter adap = new SqlDataAdapter("Select art_name, art_date, > art_desc, auth_name," + > " '" + ResolveUrl("~/") + "Articles/' + grp_folder + '/' + cat_folder + > '/' + art_filename + '.aspx' AS art_path" + > " FROM Articles, Authors, Categories, Groups" + > " WHERE cat_id = '" + id.ToString() + "' AND art_auth_id = auth_id AND > art_cat_id = cat_id AND cat_grp_id = grp_id" + > " ORDER BY art_name", conn); > DataSet ds = new DataSet(); > adap.Fill(ds); > > // Assign datasource to grid > GridView1.DataSource = ds; > GridView1.DataBind(); > > It's working correctly but I'm new to this and have a couple of questions. > > 1. Does my SQL statement seem reasonably efficient and correctly formed? > Or am I asking for performance problems with a page that renders with > complex queries? > Not only is it ugly and hard to code, it's slow (causes compilations) and dangerous (risks SQL injection). Other SQL problems in your queries are failure to use ANSI join syntax, and failure to alias tables and qualify columns by aliases, and doing concatenation in the query (which forces you to paste in the URL). string SQL = @" SELECT a.art_name, a.art_date, a.art_desc, au.auth_name, grp.grp_folder, cat.cat_folder, a.art_filename, FROM Categories cat join Articles a on a.art_cat_id = cat.cat_id join Authors au on a.art_auth_id = au.auth_id Groups grp on grp.grp_id = cat.cat_grp_id join WHERE cat_id = @cat_id ORDER BY art.art_name "; SqlCommand cmd = new SqlCommand(SQL,con); cmd.Parameters.Add(@cat_id, id); SqlDataAdapter adap = new SqlDataAdapter(cmd); .... Then you can add a calculated column to your DataTable for art_path or add it in your grid. > 2. How can I find out if no rows were returned? Check ds.Tables[0].Rows.Count> David Thanks for the comments.
Show quote "David Browne" <davidbaxterbrowne no potted m***@hotmail.com> wrote in message news:euFfUlXUHHA.1016@TK2MSFTNGP04.phx.gbl... > > > "Jonathan Wood" <jw***@softcircuits.com> wrote in message > news:uvDFhRVUHHA.4828@TK2MSFTNGP05.phx.gbl... >> I'm writing a Website that contains the following code: >> >> // Query article data >> SqlDataAdapter adap = new SqlDataAdapter("Select art_name, art_date, >> art_desc, auth_name," + >> " '" + ResolveUrl("~/") + "Articles/' + grp_folder + '/' + cat_folder + >> '/' + art_filename + '.aspx' AS art_path" + >> " FROM Articles, Authors, Categories, Groups" + >> " WHERE cat_id = '" + id.ToString() + "' AND art_auth_id = auth_id AND >> art_cat_id = cat_id AND cat_grp_id = grp_id" + >> " ORDER BY art_name", conn); >> DataSet ds = new DataSet(); >> adap.Fill(ds); >> >> // Assign datasource to grid >> GridView1.DataSource = ds; >> GridView1.DataBind(); >> >> It's working correctly but I'm new to this and have a couple of >> questions. >> >> 1. Does my SQL statement seem reasonably efficient and correctly formed? >> Or am I asking for performance problems with a page that renders with >> complex queries? >> > No. Or rather, who can tell when you splice together C# and SQL like > that. Not only is it ugly and hard to code, it's slow (causes > compilations) and dangerous (risks SQL injection). Other SQL problems in > your queries are failure to use ANSI join syntax, and failure to alias > tables and qualify columns by aliases, and doing concatenation in the > query (which forces you to paste in the URL). > > > string SQL = @" > SELECT > a.art_name, > a.art_date, > a.art_desc, > au.auth_name, > grp.grp_folder, > cat.cat_folder, > a.art_filename, > FROM > Categories cat > join Articles a > on a.art_cat_id = cat.cat_id > join Authors au > on a.art_auth_id = au.auth_id > Groups grp > on grp.grp_id = cat.cat_grp_id > join > WHERE cat_id = @cat_id > ORDER BY art.art_name > "; > > SqlCommand cmd = new SqlCommand(SQL,con); > cmd.Parameters.Add(@cat_id, id); > SqlDataAdapter adap = new SqlDataAdapter(cmd); > ... > > > Then you can add a calculated column to your DataTable for art_path or add > it in your grid. > >> 2. How can I find out if no rows were returned? >> > > Check ds.Tables[0].Rows.Count > > David > > > >
Show quote
On Thu, 15 Feb 2007 15:52:36 -0700, "Jonathan Wood" <jw***@softcircuits.com> Both previous answers are correct regarding avoidance of SQL injection attacks.wrote: >I'm writing a Website that contains the following code: > >// Query article data >SqlDataAdapter adap = new SqlDataAdapter("Select art_name, art_date, >art_desc, auth_name," + > " '" + ResolveUrl("~/") + "Articles/' + grp_folder + '/' + cat_folder + '/' >+ art_filename + '.aspx' AS art_path" + > " FROM Articles, Authors, Categories, Groups" + > " WHERE cat_id = '" + id.ToString() + "' AND art_auth_id = auth_id AND >art_cat_id = cat_id AND cat_grp_id = grp_id" + > " ORDER BY art_name", conn); >DataSet ds = new DataSet(); >adap.Fill(ds); > >// Assign datasource to grid >GridView1.DataSource = ds; >GridView1.DataBind(); > >It's working correctly but I'm new to this and have a couple of questions. > >1. Does my SQL statement seem reasonably efficient and correctly formed? Or >am I asking for performance problems with a page that renders with complex >queries? > >2. How can I find out if no rows were returned? > >Thanks! Since you say you are new at this I recommend you try to write your source code so it is easier to follow (do some formatting for readability). A year from now when you, or someone else has to read it for debugging or modification purposes, you will be happy you did. It is not my intention to flame you. I'm just trying to be helpful. Good luck with your project, Otis Mukinfus http://www.otismukinfus.com http://www.arltex.com http://www.tomchilders.com http://www.n5ge.com Otis,
> Since you say you are new at this I recommend you try to write your source I'm new to C#, ASP.NET, and haven't worked that much with SQL. But I've been > code > so it is easier to follow (do some formatting for readability). A year > from now > when you, or someone else has to read it for debugging or modification > purposes, > you will be happy you did. programming for 20 years. Other than the complexity of the query, which would be substantially improved if I moved it to a stored procedure, I'm not sure what your complaint is about the formatting. (Note that additional line breaks were inserted in the newsgroup post so it may look differently there than in my code.) > Good luck with your project, Thanks. |
|||||||||||||||||||||||
Other interesting topics