0

I am attempting to do the following:

  1. Check to see if the table does not exist and if so, create the TABLE 'tmpTriangleTransfer'.
  2. Check to see if the table exists and if so, DROP the TABLE 'tmpTriangleTransfer'.
  3. Insert the data being pulled from the other tables into the 2nd - 5th columns of the TABLE 'tmpTriangleTransfer'.
  4. Loop and for each row that exists in the TABLE 'tmpTriangleTransfer' update the 1st column with the declared information.
  5. Return all of the information from that table (to be formatted into a report).

Can someone please help me figure out what I am doing wrong? I'm getting no results even though I know for a fact there are records (when I run just the SELECT statement on the last line, it shows records and when I run the SELECT DISTINCT statement in the middle, it shows the same records).

IF OBJECT_ID('tmpTriangleTransfer') IS NOT NULL
 DROP TABLE tmpTriangleTransfer;
IF OBJECT_ID('tmpTriangleTransfer') IS NULL

CREATE TABLE tmpTriangleTransfer
            (
            CompanyName varchar(max),
            OrderID decimal(19,2) NULL,
            DriverID int NULL,
            VehicleID int NULL,
            Phone varchar(50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
            BOL varchar(50) COLLATE SQL_Latin1_General_CP1_CI_AS NULL,
            );

INSERT INTO tmpTriangleTransfer (OrderID, BOL, DriverID, VehicleID, Phone)
SELECT DISTINCT tblOrder.OrderID AS OrderID, tblOrder.BOL AS BOL, tblOrderDrivers.DriverID AS DriverID, tblDrivers.VehicleID AS VehicleID, tblWorker.Phone AS Phone
    FROM tblOrder WITH (NOLOCK)
        INNER JOIN tblActiveOrders
        ON tblOrder.OrderID = tblActiveOrders.OrderID
        INNER JOIN tblOrderDrivers
        ON tblOrder.OrderID = tblOrderDrivers.OrderID
        INNER JOIN tblDrivers
        ON tblOrderDrivers.DriverID = tblDrivers.DriverID
        INNER JOIN tblWorker
        ON tblDrivers.WorkerID = tblWorker.WorkerID
        WHERE tblOrder.CustID = 7317
        ORDER BY tblOrder.OrderID`


DECLARE @MaxRownum INT
SET @MaxRownum = (SELECT MAX(OrderID) FROM tmpTriangleTransfer)

DECLARE @Iter INT
SET @Iter = (SELECT MIN(OrderID) FROM tmpTriangleTransfer)

WHILE @Iter <= @MaxRownum
BEGIN

   UPDATE tmpTriangleTransfer
      SET tmpTriangleTransfer.CompanyName = 'Triangle'
   WHERE tmpTriangleTransfer.CompanyName IS NULL;

   SET @Iter = @Iter + 1

END

SELECT * from tmpTriangleTransfer WITH (NOLOCK)
Nathan Griffiths
  • 12,277
  • 2
  • 34
  • 51
  • 1
    Can you explain what you mean by "I'm getting no results"? It sounds like if you run the final select statement there are rows in the table, so it appears the code is working. – Nathan Griffiths May 10 '17 at 00:09
  • What is the purpose of the `WHILE` loop? It executes the same query on the same data every time, so there's no point to the loop. – Dai May 10 '17 at 00:11
  • Why are you running that update statement multiple times in a `while` loop? For that matter, why drop and recreate the table instead of just `truncate` or `delete * from...`? – Paul Abbott May 10 '17 at 00:12
  • Also, try to write SQL as though it were a functional language - avoid loops and branches. – Dai May 10 '17 at 00:12
  • Also, since you never insert any data into CompanyName it will always be NULL and so the WHILE loop is redundant. You can simply select the string 'Triangle' as part of the insert statement e.g. INSERT INTO tmpTriangleTransfer (CompanyName, OrderID, BOL, DriverID, VehicleID, Phone) SELECT DISTINCT 'Triangle' AS CompanyName, tblOrder.OrderID AS OrderID, tblOrder.BOL AS BOL.... – Nathan Griffiths May 10 '17 at 00:15
  • I want the exact same data in the CompanyName field for each row of data pulled from the other tables. If you know a better way to do this, please tell me. – Suzanne Thompson May 10 '17 at 00:15
  • see my comment above for how to select the company name as part of the main query. – Nathan Griffiths May 10 '17 at 00:17
  • @Nathan I tried your suggestion, to insert the string 'Triangle' as part of the insert statement e.g. INSERT INTO tmpTriangleTransfer (CompanyName, OrderID, BOL, DriverID, VehicleID, Phone) SELECT DISTINCT 'Triangle' AS CompanyName, tblOrder.OrderID AS OrderID, tblOrder.BOL AS BOL... and it returns an error now because it is NOT 'distinct' where all the others are. – Suzanne Thompson May 10 '17 at 00:20
  • That SQL should work, so what is the exact error you are getting? – Nathan Griffiths May 10 '17 at 00:24
  • @PaulAbbott Yes, that part was going to get modified, it was still set to DELETE because it originally was going to be in a temporary #table. – Suzanne Thompson May 10 '17 at 00:25
  • @Dai see my responses to Nathan. – Suzanne Thompson May 10 '17 at 00:25
  • @Nathan The following section, specifically line 2: `INSERT INTO tmpTriangleTransfer (CompanyName, BOL, DriverID, VehicleID, Phone) SELECT DISTINCT 'Triangle' AS CompanyName, tblOrder.OrderID AS OrderID, tblOrder.BOL AS BOL, tblOrderDrivers.DriverID AS DriverID, tblDrivers.VehicleID AS VehicleID, tblWorker.Phone AS Phone` with all the rest under it returns the following: The select list for the INSERT statement contains more items than the insert list. The number of SELECT values must match the number of INSERT columns. – Suzanne Thompson May 10 '17 at 00:26
  • @Nathan my typo found. All changes made- truncate being used at beginning of SP, took out loop, added 'Triangle' to INSERT and SELECT statements. Still not returning data even though there is data in the table which was the entire first issue that I came here with. – Suzanne Thompson May 10 '17 at 00:39
  • So, just to clarify: when you run the *entire script* there are no rows returned, but if (after running the entire script) you highlight and run only the final select statement, this returns some rows? Is that correct? If so, I can't explain that behaviour based on what you've shown so far. That said, as per Dai's answer below, this query is unnecessarily complex and the desired result can be achieved in single select statement. I recommend you use that instead. – Nathan Griffiths May 10 '17 at 00:43
  • @SuzanneThompson I suspect the problem is your use of `INNER JOIN`. Please see my answer below. – Dai May 10 '17 at 00:44

2 Answers2

2

Your existing query is far too complicated. In fact, you don't need a temporary table, the WHILE loop, or anything - just a single SELECT is all you need:

SELECT
    'Triangle' AS CompanyName,
    tblOrder.OrderId,
    tblOrder.BOL,
    tblOrderOrders.DriverID,
    tblDrivers.VehicleID,
    tblWorker.Phone
FROM
    tblOrder
    OUTER JOIN tblActiveOrders ON tblOrder.OrderID = tblActiveOrders.OrderID
    OUTER JOIN tblOrderDrivers ON tblOrder.OrderID = tblOrderDrivers.OrderID
    OUTER JOIN tblDrivers      ON tblOrderDrivers.DriverID = tblDrivers.DriverID
    OUTER JOIN tblWorker       ON tblDrivers.WorkerID = tblWorker.WorkerID
WHERE
    tblOrder.CustID = 7317
ORDER BY
    tblOrder.OrderID
  • I've changed your query to use OUTER JOIN instead of INNER JOIN because I suspect this is the main reason for no data being returned. INNER JOIN requires rows to exist in both tables (relations) and I suspect that you have Orders without Drivers or that not every Order is in ActiveOrders. Change the joins to INNER JOIN if you know that related rows will always be present.
  • You can return literals in queries directly, like I'm doing in the SELECT 'Triangle' AS CompanyName part, whereas you were seemingly manually adding it to the output temporary-table.
  • Your code didn't seem to be doing anything that would require the WITH (NOLOCK) modifier - the fact it was repeated everywhere makes it look like a case of Cargo-Cult Programming.
  • Tip: In SQL, a SELECT statement, as written, is not representative of its logical execution order. It should instead be read in this order: FROM > WHERE > [GROUP BY >] SELECT > ORDER BY.
    • This is why in .NET Linq the .Select() call is often at the end, not the beginning, because previous Linq expressions define the data sources.
  • This query can be parameterised by converting it to a Table-defined Function that accepts CustID as a parameter, I also assume you have the company name "Triangle" stored in a table somewhere - embedding it as a literal value for a single query is a code-smell - what's so special about 7317 / "Triangle"?
    • Related note: Generally speaking, queries that only SELECT data (and don't perform any INSERT/UPDATE/DELETE/ALTER/CREATE statements) should be Table-valued UDFs or Views and not Stored Procedures - so that they can benefit from function-composition, query-composition and runtime execution plan optimizations that you cannot get with Stored Procedures.
  • If you're able to, see if you can remove the tbl prefix from the table names (Using "tbl" as a prefix has its defenders, but my own personal opinion is that it's an obsolete developer aid as today's database tooling shows type information, and it makes database refactoring harder (e.g. converting a table to a view).
Dai
  • 141,631
  • 28
  • 261
  • 374
  • 1. Checking into it. 2. Fixed 3. Required by employer 4. Required by customer 5. Required by employer FYI This is an SP for a custom report. Nathan got that correct. – Suzanne Thompson May 10 '17 at 00:46
  • I broadly agree with your answer but some of your comments regarding "code smells", using table-valued UDFs and modifying table names are probably a bit off-topic - this doesn't appear to be a query that is part of a software application, it looks like an ad-hoc query being run to output data for a report. – Nathan Griffiths May 10 '17 at 00:47
  • Can you please provide a link or elaborate on `Generally speaking, queries that only SELECT data (and don't perform any INSERT/UPDATE/DELETE/ALTER/CREATE statements) should be Table-valued UDFs or Views and not Stored Procedures `. I am very curious about that. – CodingYoshi May 10 '17 at 00:53
  • @CodingYoshi I'm not sure how to further elaborate given I already explained that when queries exist in UDFs or Views that they benefit from query-composition but when they're in a Sproc they don't get that benefit. – Dai May 10 '17 at 00:58
  • Sorry I don't mean to sound as if I am attacking your answer but you are making very bold statements and i want to read up more on them. For example, where can I look more into `UDFs or Views that they benefit from query-composition but when they're in a Sproc they don't get that benefit. ` – CodingYoshi May 10 '17 at 01:08
  • @CodingYoshi See other SO QAs such as http://stackoverflow.com/questions/1942753/performance-difference-between-user-defined-function-and-stored-procedures and http://stackoverflow.com/questions/178128/functions-vs-stored-procedures - in short: functions are composable, sprocs are not. – Dai May 10 '17 at 01:48
0

Taken from a combination of the suggestion from Dai and the requirements of my employer:

`SELECT 'Triangle' AS CompanyName, tblOrder.OrderId AS OrderID, tblOrder.BOL AS BOL, tblOrderDrivers.DriverID AS DriverID, tblDrivers.VehicleID AS VehicleID, tblWorker.Phone AS Phone
FROM tblOrder WITH (NOLOCK)
        INNER JOIN tblActiveOrders WITH (NOLOCK)
        ON tblOrder.OrderID = tblActiveOrders.OrderID
    INNER JOIN tblOrderDrivers WITH (NOLOCK)
        ON tblOrder.OrderID = tblOrderDrivers.OrderID
    INNER JOIN tblDrivers WITH (NOLOCK)
        ON tblOrderDrivers.DriverID = tblDrivers.DriverID
    INNER JOIN tblWorker WITH (NOLOCK)
        ON tblDrivers.WorkerID = tblWorker.WorkerID
WHERE
    tblOrder.CustID = 7317
ORDER BY
    tblOrder.OrderID desc`
Eric Aya
  • 69,473
  • 35
  • 181
  • 253