Please start any new threads on our new
site at https://forums.sqlteam.com. We've got lots of great SQL Server
experts to answer whatever question you can come up with.
| Author |
Topic |
|
BorgPidgeon
Starting Member
4 Posts |
 Posted - 2007-09-24 : 06:05:13
|
| Hi,I'm trying to select information from two tables using an OUTER JOIN as there will not always be a record in the second table. The problem is that the second table holds data for multiple users and I can't bring back information for just one user in this way.SELECT News.nAuthor, News.nSubject, News.nDate, News.nMessage, News.ID, IsNull(NewsReciept.NewsID,'0') as ""Read"" FROM News LEFT OUTER JOIN NewsReciept ON News.ID = NewsReciept.NewsID WHERE NewsReciept.AgentName = '" & uName & "' AND News.nSubject NOT LIKE '** N%' ORDER BY nDate DESC;Table 1 - Contains newsTable 2 - Contains date time, news ID and the agent's nameAn entry goes into Table 2 when an agent clicks "I have read this" on the news post. But this is only brings back the news items the agent has clicked "I have read this" for.I need to be able to bring back all news posts and know which the agent has clicked "I have read this" on.Thank you, all help is appreciated! |
|
|
Kristen
Test
22859 Posts |
Posted - 2007-09-24 : 06:14:25
|
Move the constraint on the NewsReciept table OUT of the WHERE and INTO the JOIN. That way it only applies to the JOIN'd data, and not to the resultset as a wholeOUTER JOIN NewsReciept ON News.ID = NewsReciept.NewsID WHERE AND NewsReciept.AgentName = '" & uName & "' Kristen |
 |
|
|
Kristen
Test
22859 Posts |
Posted - 2007-09-24 : 06:31:10
|
Couple of suggestions:IsNull(NewsReciept.NewsID,'0') as ""Read"" the double'd up quotes are, presumeably, because this is dynamic SQL constructed in your application language?If so you might prefer to use [XXX] for column names:IsNull(NewsReciept.NewsID,'0') as [Read]which you can also write as[Read] = IsNull(NewsReciept.NewsID,'0')if you prefer. (Its my preference because on long lines of SQL I get to see the alias-name of the column, which might be "lost" if I use the "AS xxx" on the end of a long line!); the [...] won't need the double-quotingIsNull(NewsReciept.NewsID,'0')I recommend that you use COALESCE instead of ISNULL. Also, assuming that NewsReciept.NewsID is an INT datatype, rather than Varchar/Text-string, you should use:COALESCE/IsNull(NewsReciept.NewsID,0) (no apostrophes)so that SQL explicitly understands the second parameter to be a number, rather than implicitly converting it from String to Number (which, as a second parameter to IsNull, has potential problems - and, again, which COALESCE does not suffer from, but that's all a but pedantic and by-the-by  )WHERE NewsReciept.AgentName = '" & uName & "' Like the above, I assume this is syntax from you application language, and represents a string concatenation? You should NEVER do this! At the very least you need to pre-process the uName parameter with some function to double-up any embedded apostrophes, otherwise you leave yourself wide open to SQL Injection hacker attacksSee: http://www.sqlteam.com/forums/topic.asp?TOPIC_ID=55210&SearchTerms=SQL+Injectionand for performance reasons (and also to combat SQL Injection) you would be better off using a parameterized query, rather than constructing straight dynamic SQL in this way. If your application is small and doesn't have many users or records this may not be of too much concern.See: http://weblogs.sqlteam.com/jeffs/archive/2006/07/21/10728.aspxand maybe: http://www.sqlteam.com/forums/topic.asp?TOPIC_ID=70231Kristen |
|
|
|
BorgPidgeon
Starting Member
4 Posts |
Posted - 2007-09-24 : 06:48:07
|
| Kristen, you are fantastic. Thank you very much for your help! I honestly didn't think it was possible and was just posting as a last resort before having to do some funky coding.You are absolutely right about the double quotes and I've changed it to the square-brackets. I'm using ISNULL() because if there is no ID in there then it's not been read I don't actually do anything with the ID if it isn't null.-----If RSNews("Read") = "0" Then Response.write("<br /><form action=""newbs.asp"" method=""post""><input type=""hidden"" name=""nID"" value=""" & RSNews("ID") & """/><input type=""submit"" name=""reciept"" value=""I have read this"" /></form>")End If-----Yes it's classic ASP but it still works :DThanks a lot for your help.EDIT:/Regarding string concatenation - I always replace single apostrophes with double apostrophes, I just didn't include the full source but thanks for mentioning it anyway :) |
|
|
|
Kristen
Test
22859 Posts |
Posted - 2007-09-24 : 06:53:49
|
| "Thank you very much for your help"No problem."I'm using ISNULL() because if there is no ID in there then it's not been read I don't actually do anything with the ID if it isn't null."Understood, but you should still watch out for the mixed datatypes, and apart from the rather ghastly name I think you should use the COALESCE() function rather than IsNull().Kristen |
|
|
|
madhivanan
Premature Yak Congratulator
22864 Posts |
Posted - 2007-09-25 : 04:32:48
|
| <<Its my preference because on long lines of SQL I get to see the alias-name of the column, which might be "lost" if I use the "AS xxx" on the end of a long line!>>I prefer using "As xxx" because I think using xxx="something" seems to be assigning value to a column in a update statement. Also I am not sure if all RDBMSs support xxx="something" approach when aliasingMadhivananFailing to plan is Planning to fail |
|
|
|
Kristen
Test
22859 Posts |
Posted - 2007-09-25 : 04:40:38
|
"I prefer using "As xxx" because I think using xxx="something" seems to be assigning value to a column in a update statement"Yes, I know what you mean, although I consider it "assigning to the result set", so that perhaps would fit with your analogy?"Also I am not sure if all RDBMSs support xxx="something" approach when aliasing"That's my understanding too, but there is no way my Sprocs are going to run on anything other than SQL Server, so I've stopped worrying about it  Kristen |
|
|
|
madhivanan
Premature Yak Congratulator
22864 Posts |
Posted - 2007-09-25 : 08:18:13
|
| <<That's my understanding too, but there is no way my Sprocs are going to run on anything other than SQL Server, so I've stopped worrying about it>>Anyway, Use proper ANSI style syntax MadhivananFailing to plan is Planning to fail |
|
|
|
jsmith8858
Dr. Cross Join
7423 Posts |
Posted - 2007-09-25 : 08:40:08
|
quote:
Regarding string concatenation - I always replace single apostrophes with double apostrophes, I just didn't include the full source but thanks for mentioning it anyway :)
Be sure to read the article about using parameters carefully ( http://weblogs.sqlteam.com/jeffs/archive/2006/07/21/10728.aspx )-- it is not just about replacing single apostrophes with doubles, it involves delimiters and date formatting as well. It makes your code shorter, faster, easier to read and to write, safer, and more efficient to use parameters. You should always, always use them and never, ever concatenate values into a sql statement as you are doing.- Jeffhttp://weblogs.sqlteam.com/JeffS |
|
|
|
BorgPidgeon
Starting Member
4 Posts |
Posted - 2007-09-25 : 08:59:08
|
| Thank you for your comments about parameters but I am not using them. They are a .NET feature which doesn't appear to be available in classic ASP which this project uses.I am quite happy to look at long pages of SQL and deal with them the proper way than fiddling with lots of complicated parameters. The whole .NET approach to database work is slow and visually unattractive. My code works, works well and works quickly... unless someone can give me a better reason than "Because Microsoft said so" I think I'll stick with the traditional way. |
|
|
|
jsmith8858
Dr. Cross Join
7423 Posts |
Posted - 2007-09-25 : 09:21:49
|
| parameters are not a .NET feature, and not MS-only. they are available in classic ASP and ADO and in Java and Oracle and pretty much any other database interface library on all platforms, since they are universally considered best practices and the proper way to incorporate input into your sql statements. They are not complicated, and as I demonstrated in the article, they actually make your code much *simpler* and easier to read, write and work with, and you are passing things to your database by *value* and not converting EVERY SINGLE VALUE that you send to a STRING, which the database must then convert BACK to the correct data types.You are not doing things the "proper" way, you are doing things the wrong way -- no good professional programmer with any experience at all in this with agree with you on that.Did you even read the article? Not one reason given to use parameters is "because MS said so", it is all broken down into formatting, delimiting, security, escaping, readability, performance, and so on ... all of which are easily solved simply by using parameters ... It's extra ironic because you are experiencing many of those problems right now as demonstrated in this thread ... - Jeffhttp://weblogs.sqlteam.com/JeffS |
|
|
|
BorgPidgeon
Starting Member
4 Posts |
Posted - 2007-09-25 : 09:37:34
|
| Hello jsmith8858,Thank you for your response. It is very much appreciated, fortunately the project has already been completed and is functioning correctly. Unfortunately I will not keep responding to this discussion as I will not be drawn into a debate about SQL which, as I understand it, is string based. Should you have any further comments, I am sure there is another topic or forum which would benefit greatly from your experience.Once again I thank Kristen for his excellent and very helpful responses, this is most certainly one of the best technical forums around. |
|
|
|
X002548
Not Just a Number
15586 Posts |
Posted - 2007-09-25 : 09:42:14
|
quote:
Originally posted by BorgPidgeon
Hello jsmith8858,Thank you for your response. It is very much appreciated, fortunately the project has already been completed and is functioning correctly. Unfortunately I will not keep responding to this discussion as I will not be drawn into a debate about SQL which, as I understand it, is string based.
Holy sheet...I'm laughing so hard I fell off my bar stoo...umm office chair, and I have to clean coffee spew of my monitorBrett8-)Hint: Want your questions answered fast? Follow the direction in this linkhttp://weblogs.sqlteam.com/brettk/archive/2005/05/25/5276.aspxAdd yourself!http://www.frappr.com/sqlteam |
|
|
|
jsmith8858
Dr. Cross Join
7423 Posts |
Posted - 2007-09-25 : 09:43:35
|
quote:
Unfortunately I will not keep responding to this discussion as I will not be drawn into a debate about SQL which, as I understand it, is string based.
I guess that says it all! That is the quote of the year!!!Unfortunately, you don't "understand it" at all .... - Jeffhttp://weblogs.sqlteam.com/JeffS |
|
|
|
X002548
Not Just a Number
15586 Posts |
|
|
Kristen
Test
22859 Posts |
Posted - 2007-09-26 : 05:20:21
|
| Yeah, well I take a different view here chaps.I definitely agree with Jeff that parameters are the Best Practice.We rarely use them though.Through the route that we have chosen to build our "interface" between our application and SQL Server it is very much easier for us to construct string concatenated commands to SQL Server.(All our string parameters are double-single-quoted to prevent SQL injection and, as it happens, all our string-SQL are calls to Sprocs)If I was doing this all again, with what I know now, I would probably insist on the parameters-route, but at the time the performance tests we did suggested less than 5% improvement for "proper" parameterized Sprocs, and the development cost to us would have been greater (in terms of programmer-hours).Our particular application might also have had to query what parameters are available for an Sproc, which in turn would have meant a round-trip to discover them, at run time, which probably would have killed off the 5% advantage - or we would have had to build a Caching-system to manage that info and prevent the round-trip.So I don't think the OP is as "wrong" in his approach as you portray, notwithstanding the fact that I do think that your approach represents Best practice.I also feel strongly that folk that turn up here wanting a solution to a specific problem should be alerted to anything they may be doing which is NOT Best Practice, as that may be the biggest favour we can do for them long term! And the OP has definitely gone away with that knowledge. But having an expectation that a whole project is going to be rewritten, late in its development cycle, ain't going to wash either. The OP has been very polite in pointing that out, which can often be rare amongst posters here too.Right, lets draw up two battle lines then ... I'll run and get my flame-proof underwear Kristen |
|
|
|
jsmith8858
Dr. Cross Join
7423 Posts |
Posted - 2007-09-26 : 09:00:06
|
Kristen -- I hear what you are saying, but you really should read my weblog post. It is shorter, quicker, and easier to use parameters. Your SQL can be constructed much easier with meaningful parameter name placeholders, without string concatenations, making your sql strings much, much easier to read, write and debug as opposed to wading through long, messy, convoluted string concatenations. In fact you can store your SQL strings elsewhere since they have nice parameter place holders, if necessary.There is never a need to do round-trips to discover parameters, especially when the SQL is constructed at the client, I am not sure why you would think there is.I never expect someone to re-do their entire project if they don't time or resources, but you should certainly acknowledge the benefits of parameters and learn from others and be sure to use them going forward -- for that, there is never an argument not to. And, in fact, I've had great success rewriting apps and getting rid of crappy, buggy, sql-injectionable concatenated sql strings and replacing them with either stored procs and/or parameterized sql commands. As soon as that one time pain of doing this was done, further maintainance of that application become so much easier and it worked so much better that the day or two of overhead was well worth it and proved to be many times over. quote:
And the OP has definitely gone away with that knowledge
Unfortunately, no, it doesn't appear that he has, based on his responses ....It's not so much that the OP is using concatenation now or has in the past, it is that he refused to acknowledge or recognize how he can improve his code going forward, and he also demonstrated a clear lack of understanding of how sql commands work by thinking that "everything is string-based".- Jeffhttp://weblogs.sqlteam.com/JeffS |
|
|
|
Kristen
Test
22859 Posts |
Posted - 2007-09-26 : 10:54:03
|
| Yup, I have read your blog and do agree with it."It is shorter, quicker, and easier to use parameters. Your SQL can be constructed much easier with meaningful parameter name placeholders, without string concatenations, making your sql strings much, much easier to read, write and debug as opposed to wading through long, messy, convoluted string concatenations"Not the way we have done it. But I doubt that applies to your casual SQL-string-concatenating developer I must admit.We are using meta data from tables which form a super-set to the data SQL server stores about tables & columns, and mechanically generating calls from that. The coding for generating AddParameter type stuff would have been more complex at the time we built it. We did look into it, discovered a 5% performance advantage and took a conscious decision that the additional development time was not warranted for the performance gain.Right now, having my time over again, I would change the meta data system and the way we generate the code."There is never a need to do round-trips to discover parameters, especially when the SQL is constructed at the client, I am not sure why you would think there is."We do have some parameter generating code.For example, I can write an SProc that requires @UserID and, lets assume, that @UserID is a field on a form.I set up our metadata for that page's form and tell it that it should post to "MySproc", but it has no further knowledge of what MySproc needs, or can do.Our "parameterising" stuff (as compared to the "non-parameterising" stuff, which we are talking about above) will interrogate the SProc, discover it uses @UserID, detect that data is in the Request object, and pass the information from the form to the Sproc - using the appropriate AddParameter / datatype / yada-yada.Lets say we now have a security issue, and we want to change the Sproc to perform differently based on what the current Site is, or the User's Permissions, or somesuch. In fact anything which is either on the Form for that page, or in what might be termed "session" data.So I add another parameter to MySproc (@SiteName, @UserPermisson, whatever) and our system will automatically provide the matching data to it from the form or session-state data. (No application code changes).That requires that we either interrogate the SProc to see what its parameters are, or keep some sort of meta data or cache.Which would be fine, but we didn't think of / do that at the time, and went down a different evolution process for the bulk of our code, based on the Meta tables mentioned above." sql-injectionable concatenated sql strings"I have no evidence that concatenated sql strings with doubled-single-quotes has any ability to be injected, but I would sure like to hear of any!Kristen |
|
|
|
jsmith8858
Dr. Cross Join
7423 Posts |
Posted - 2007-09-26 : 11:27:08
|
quote:
We do have some parameter generating code.For example, I can write an SProc that requires @UserID and, lets assume, that @UserID is a field on a form.
Ah, that is a different story. That is not dynamic sql, that is a form that is "bound" to an existing stored procedure that is querying the proc to see what parameters it expects to dynamically provide them ... that's a completely different scenario than whether or not to concatenate sql strings with input or to use parameters when dynamically creating sql at the client.- Jeffhttp://weblogs.sqlteam.com/JeffS |
|
|
|
Kristen
Test
22859 Posts |
Posted - 2007-09-26 : 11:31:52
|
| "that's a completely different scenario than whether or not to concatenate sql strings with input or to use parameters when dynamically creating sql at the client"Indeed. And I do agree that for normal use the parameterized route is better, safer, faster, and provides more job-security and attractive partners! |
|
|
|
|
|
|
|
|
OUTER JOIN problem