October 23, 2008 at 2:43 pm
I am having issues with this query when binding it to my DataTable.
protected void fillErUp()
{
string connectionString = ConfigurationManager.ConnectionStrings["SQLServerConnectionString"].ConnectionString;
StringBuilder sqlString = new StringBuilder();
sqlString.Append("SELECT m.ID as displayID, isnull(fname,lastname) + " + "' " + "' " + " + lastname as displayName from HaitiMailingList");
//Donor Giving Search comes from the Donations DB
//if any of the 3 fields GivingLow, GivingHi or YearToReport have a value all 3 must.
if (txtGivingLow.Text != String.Empty && txtGivingHi.Text != String.Empty)
{
sqlString.Append(" JOIN Donations e on m.ID = e.ID and ");
sqlString.Append(" ([Receipt Date] >='01/01/" + donorYearToReport + "' ");
sqlString.Append(" and [Receipt Date] <='12/31/" + donorYearToReport + "' )");
sqlString.Append(" GROUP BY m.ID, Title, fname, lastname ");
sqlString.Append(" HAVING sum(Amount) >= '" + txtGivingLow.Text + "'");
sqlString.Append(" and sum(Amount) <= '" + txtGivingHi.Text + "'");
}
SqlDataAdapter da = new SqlDataAdapter(sqlString.ToString(), connectionString);
DataTable ds = new DataTable("Donors");
da.Fill(ds);
grdSearch.DataSource = ds;
grdSearch.DataBind();
}
It runs fine in SQL Server but won't bind to the DataTable.
I get the error:The multi-part identifier "m.ID" could not be bound. Any help?
October 23, 2008 at 3:04 pm
I'm guessing that it's because the alias to the first table is missing
sqlString.Append("SELECT m.ID as displayID, isnull(fname,lastname) + " + "' " + "' " + " + lastname as displayName from HaitiMailingList m");
If not, print the value of the SQLString to debug and post it here.
Not related, but still important. Google SQL Injection and read up on it. Your code is vulnerable.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
October 23, 2008 at 3:43 pm
Thanks Gila that was exactly the issue. I hate it when it is something like that.
To the other point. Yes I understand injection but this is an intranet application that will never see the internet so I think I am covered.
Thanks again.
October 23, 2008 at 4:27 pm
john (10/23/2008)
Thanks Gila that was exactly the issue. I hate it when it is something like that.To the other point. Yes I understand injection but this is an intranet application that will never see the internet so I think I am covered.
Thanks again.
Really - you have no users who could potentially cause you problems? Are you absolutely sure there is no way to access this outside of your approved methods?
And finally, what happens when this application is pushed to the web because your end users want to be able to use it from home?
Just because you think you are protected does not mean you should abandon best practices. It is not any harder to write an application correctly than it is to write it badly - so, why write it badly in the first place? I guess I just don't understand that mindset.
Jeffrey Williams
“We are all faced with a series of great opportunities brilliantly disguised as impossible situations.”
― Charles R. Swindoll
How to post questions to get better answers faster
Managing Transaction Logs
October 23, 2008 at 5:00 pm
Ouch! That is harsh. I guess I am writing it badly because I don't know all the details on how to write it better and it is for a non-profit who is not paying me to do it.
Yes I would love to learn how to write it better but I am still a noob and learning.
October 23, 2008 at 5:20 pm
Sorry for coming across as harsh - but, it is better that you learn how to avoid this now than to be bitten later. Especially in this situation where you probably will not know when (if?) the users decide to expose this to the internet.
There are plenty of articles around that will show you how to avoid this and I would suggest that you do the research necessary to avoid this situation.
Again, with not too much effort you can rewrite this to avoid the issue. It is much easier to do this now, than to try to change it after it has been in use for some time.
Jeffrey Williams
“We are all faced with a series of great opportunities brilliantly disguised as impossible situations.”
― Charles R. Swindoll
How to post questions to get better answers faster
Managing Transaction Logs
October 23, 2008 at 9:46 pm
john (10/23/2008)
Ouch! That is harsh. I guess I am writing it badly because I don't know all the details on how to write it better and it is for a non-profit who is not paying me to do it.Yes I would love to learn how to write it better but I am still a noob and learning.
Well, I am a little rusty with C# and ADO.net, but what you want is something like this:
protected void fillErUp()
{
string connectionString = ConfigurationManager.ConnectionStrings["SQLServerConnectionString"].ConnectionString;
StringBuilder sqlString = new StringBuilder();
sqlString.Append("SELECT m.ID as displayID, isnull(fname,lastname) + ' ' + lastname as displayName from HaitiMailingList m");
//Donor Giving Search comes from the Donations DB
//if any of the 3 fields GivingLow, GivingHi or YearToReport have a value all 3 must.
if (txtGivingLow.Text != String.Empty && txtGivingHi.Text != String.Empty)
{
sqlString.Append(" JOIN Donations e on m.ID = e.ID and ");
sqlString.Append(" ([Receipt Date] >= @Year ");
sqlString.Append(" and [Receipt Date] < Dateadd(yy,1,@Year) )");
sqlString.Append(" GROUP BY m.ID, Title, fname, lastname ");
sqlString.Append(" HAVING sum(Amount) >= @Low ");
sqlString.Append(" and sum(Amount) <= @hi ");
}
SqlCommand cmd = new SqlCommand("sp_executesql();", connectionString);
cmd.CommandType = CommandType.StoredProcedure;
cmd.Parameters.Add(new SqlParameter("@stmt", sqlString.ToString()));
cmd.Parameters.Add(new SqlParameter("@params", "@Year datetime, @Low Money, @hi Money"));
cmd.Parameters.Add(new SqlParameter("@Year", new SqlDateTime(donorYearToReport, 1, 1)));
cmd.Parameters.Add(new SqlParameter("@Low", new SqlMoney(txtGivingLow.Text)));
cmd.Parameters.Add(new SqlParameter("@Hi", new SqlMoney(txtGivingHi.Text)));
SqlDataAdapter da = new SqlDataAdapter(cmd)
DataTable ds = new DataTable("Donors");
da.Fill(ds);
grdSearch.DataSource = ds;
grdSearch.DataBind();
}
[font="Times New Roman"]-- RBarryYoung[/font], [font="Times New Roman"] (302)375-0451[/font] blog: MovingSQL.com, Twitter: @RBarryYoung[font="Arial Black"]
Proactive Performance Solutions, Inc. [/font][font="Verdana"] "Performance is our middle name."[/font]
October 23, 2008 at 10:25 pm
WOW! Thank you very much. I will study it and try to write better code from now on.
October 23, 2008 at 10:39 pm
Glad I could help.
And definitely test this. I doubt very much that I got this right the first time.
[font="Times New Roman"]-- RBarryYoung[/font], [font="Times New Roman"] (302)375-0451[/font] blog: MovingSQL.com, Twitter: @RBarryYoung[font="Arial Black"]
Proactive Performance Solutions, Inc. [/font][font="Verdana"] "Performance is our middle name."[/font]
October 24, 2008 at 12:48 am
[font="Verdana"]John, Stored Procedure would have been best option then. You could pass just parameters from front end(shown by rbarryyoung)
--Mahesh[/font]
MH-09-AM-8694
October 24, 2008 at 3:09 am
john (10/23/2008)
Yes I would love to learn how to write it better but I am still a noob and learning.
Best practice - all data access via stored procedures. Parametrise the calls to those stored procedures (similar to how rbarry showed). Grant no access to the base tables, just execute rights on the stored procedures.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
October 24, 2008 at 6:14 am
I agree with Mahesh, this would be better (and easier!) as a stored procedure.
[font="Times New Roman"]-- RBarryYoung[/font], [font="Times New Roman"] (302)375-0451[/font] blog: MovingSQL.com, Twitter: @RBarryYoung[font="Arial Black"]
Proactive Performance Solutions, Inc. [/font][font="Verdana"] "Performance is our middle name."[/font]
Viewing 12 posts - 1 through 11 (of 11 total)
You must be logged in to reply to this topic. Login to reply