5
\$\begingroup\$

I am still fairly green when it comes to SQL joins. The below code works but I want to check that I am doing it the best way before I copy and paste errors into other work in the future.

The idea of the below code is to get the PropertyID and Name of a property from the first Table and using the PropertyID join them. In this case I am actually using every field in Table B (thus the *).

<asp:SqlDataSource ID="SqlDataSource1" runat="server" ConnectionString="<%$ ConnectionStrings:SQL %>"
    SelectCommand="      
        select TblA.PropertyID as PId, TblA.Propertyname as PNa, TblB.*  
        from TbPropertyDetails as TblA 
            inner join TbPropertyDetailsSafeguarding as TblB on TblA.PropertyID = TblB.PropertyID 
       where TblA.RegionID <> '0' and TblA.PropertyID LIKE '%' + @PropertyID + '%'  
       order by TblA.PropertyID asc;"
>

<SelectParameters>
    <asp:SessionParameter DefaultValue="" Name="PropertyID" SessionField="PropertyID"
        Type="string" />
</SelectParameters>
</asp:SqlDataSource>

I am programming in VB.net but this is really standalone SQL.

\$\endgroup\$
  • \$\begingroup\$ Do you mean Microsoft SQL? If so that is the one I am using, do you need more detail \$\endgroup\$ – indofraiser Jan 2 '14 at 14:11
  • \$\begingroup\$ Do you want to show all properties or just those which are in the TblB too? \$\endgroup\$ – PeterRing Jan 2 '14 at 14:37
  • \$\begingroup\$ Have you tested this code? \$\endgroup\$ – Malachi Jan 2 '14 at 14:42
  • 1
    \$\begingroup\$ Yes, it works and filters correctly. The SessionParameter relates to @PropertyID \$\endgroup\$ – indofraiser Jan 2 '14 at 14:47
5
\$\begingroup\$

In this small of a query I wouldn't alias anything. It doesn't make it any more readable and you don't use those column aliases anywhere else in the query.

Don't do the reporting stuff on the SQL Server. you can set the title of the column in the datagrid. Don't do it here, it creates clutter if you try to do it in the SQL.

Whatever you are using to display this data is what you should use to give it a display title.

Always separate retrieving data from displaying data it makes it easier to code.


I assume that you have all of your RegionID's Positive so you could change the query from

WHERE RegionID <> '0'

to

WHERE RegionID > 0

I hope that RegionID is an Integer in which case the ' are not needed because they make it a string.


Something else that I just noticed, must have been an addition when you edited your post.

WHERE TblA.PropertyID LIKE '%' + @PropertyID + '%'

I always think of an ID field being an integer, and if that is the case then you shouldn't (and CANNOT) use Wildcards or the LIKE operator on that column. I really don't know how it is running like that.

the LIKE Operator uses a lot of performance as well, so let's say it works, why would you want to do this in the first place, if you put in 1 for @PropertyID it will bring back records with a PropertyID of

1,11,111,1111,12,13,14,15,16,17,18,19,21,31,41,51,61,71,81,91,(100 <= x < 200)... 

if you put in 13 you will get

13,113, 133,1413,1313,1334,1113,2132,1132, ...

I don't see this as being what you want to functionally happen in this query


You don't need this in the Query

ORDER BY TblA.PropertyID ASC

This should be done in the VB code as well.


This is more of a common coding practice type of thing (nitpick) all of your reserved words should be in all Caps

WHERE, JOIN, INNER, SELECT, LIKE, FROM, ORDER BY, AND, AS, ON
\$\endgroup\$
  • \$\begingroup\$ Typo correct re:alias. The alias will be used in the datagrid (not show here). The idea is to show a small example and if that is correct then I can use it as a baseline going forward. \$\endgroup\$ – indofraiser Jan 2 '14 at 14:09
  • \$\begingroup\$ don't do the reporting stuff on the SQL Server. you can set the title of the column in the datagrid. don't do it here, it's clutter. and those aren't very good Aliases anyway \$\endgroup\$ – Malachi Jan 2 '14 at 14:24
  • \$\begingroup\$ The alias is for the datafield on the datagrid. How would you do it? \$\endgroup\$ – indofraiser Jan 2 '14 at 14:26
  • \$\begingroup\$ regionID is an Integer. Nice idea, marginal gains :-) \$\endgroup\$ – indofraiser Jan 2 '14 at 14:39
  • \$\begingroup\$ Nitpik away :-) That's the whole idea. The wild card is the client specific request (I agree it's not ideal but when off site they sometimes can not remember the full site ID, what I might do it put a Site Name and wild card that and have Property ID with no wildcard). \$\endgroup\$ – indofraiser Jan 3 '14 at 10:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.