4
\$\begingroup\$

I have a table with this schema:

CREATE TABLE [td].[MyTable] (
     [ID] [int] NOT NULL 
    ,[FatherID] [int] NULL
)

(Note: I have excluded all the columns not relevant to the discussion)

I receive an [ID] as input, and I need to collect the relative record and all of its fathers in one single output. Every record has the id of the Father stored in the [FatherID] column. There is only one root element and it is recognized when we found the [FatherID] = 0.

This is the current full working query:

IF OBJECT_ID('tempdb..#MyTempTable') IS NOT NULL DROP TABLE #MyTempTable

DECLARE @ID CHAR(11)

SET @ID = 13192

SELECT *
INTO #MyTempTable
FROM [MyTable]
WHERE [ID] = @ID

SELECT @ID = [FatherID]
FROM #MyTempTable
WHERE [ID] = @ID

WHILE @ID <> 0
BEGIN
    INSERT INTO #MyTempTable
    SELECT *
    FROM [MyTable]
    WHERE [ID] = @ID

    SELECT @ID = [FatherID]
    FROM #MyTempTable
    WHERE [ID] = @ID
END

SELECT *
FROM #MyTempTable
ORDER BY [ID]

The perfomance are fine, but I want improve the readability. Any suggestions?

\$\endgroup\$
4
  • \$\begingroup\$ can you please provide some example data \$\endgroup\$
    – Malachi
    Aug 20 '14 at 21:03
  • \$\begingroup\$ The data in the Sql Fiddle created by PenutReaper represents the same concept of the data in my database. Let me know if you need any specif information. \$\endgroup\$
    – simoneL
    Aug 20 '14 at 21:15
  • \$\begingroup\$ I understand the need for discretion, but the naming here conveys no meaning at all and this is example code. As such, I believe it's off topic. \$\endgroup\$
    – RubberDuck
    Aug 20 '14 at 22:31
  • \$\begingroup\$ Use the original names would not bring any advantage to the discussion. Instead, the real table uses a pretty cryptic syntax for the names. I can guarantee it's better to talk about this question using these mock names \$\endgroup\$
    – simoneL
    Aug 20 '14 at 22:35
4
\$\begingroup\$

Firstly, you use both the

SELECT [XYZ] 
INTO [ABC]

and the

INSERT INTO [ABC] 
SELECT [XYZ] 

syntax in the same query, using one would make the query much more consistent and easier to read. Personally, I prefer using INSERT INTO [ABC] SELECT [XYZ], you do have to explicitly create the temporary table though, but that makes it usually clearer what is happening.


If you'd prefer, you can change

DECLARE @ID CHAR(11)
SET @ID = 13192

to

DECLARE @ID CHAR(11) = 13192

And you should really explicitly drop the #MyTempTable at the end of the query.


Lastly, the biggest change I would suggest is to completely remove the selects from outside of the while, they are completely redundant. If you remove those selects from outside the while loop, then the first iteration of the while does exactly the same as the selects on the outside would have done.


Here is what we end up with:

IF OBJECT_ID('tempdb..#MyTempTable') IS NOT NULL DROP TABLE #MyTempTable

DECLARE @ID CHAR(11) = 13192

CREATE TABLE #MyTempTable(ID INT, FatherID INT)

WHILE @ID <> 0
BEGIN
    INSERT INTO #MyTempTable
    SELECT *
    FROM [MyTable]
    WHERE [ID] = @ID

    SELECT @ID = [FatherID]
    FROM #MyTempTable
    WHERE [ID] = @ID
END

SELECT *

FROM #MyTempTable
ORDER BY [ID]

DROP TABLE #MyTempTable

Here is an SQL Fiddle.

\$\endgroup\$
1
  • \$\begingroup\$ Great, in this way it's enough easy to understand. Thank you also for the SQL Fiddle. \$\endgroup\$
    – simoneL
    Aug 20 '14 at 14:21
4
\$\begingroup\$

you can use recursive CTE Too

DECLARE @Id int =1;
WITH RecursiveCte AS
(
SELECT 1 as Level, H1.Id, H1.FatherId FROM [MyTable] H1
WHERE Id = @Id
UNION ALL
SELECT RCTE.level + 1 as Level, H2.Id, H2.FatherId FROM [MyTable] H2
INNER JOIN RecursiveCte RCTE ON H2.Id = RCTE.FatherId
)
SELECT*  FROM RecursiveCte

By this you will enlist all the parent upto hierarchy.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for your answer, but I think PenutReaper's suggestion is more readable. \$\endgroup\$
    – simoneL
    Aug 20 '14 at 14:55
  • 1
    \$\begingroup\$ To any developer experienced in SQL, the recursive query should be more idiomatic and easily recognizable. I highly recommend it over reinventing the wheel with your own procedure to implement iterative traversal. \$\endgroup\$ Aug 20 '14 at 16:13
1
\$\begingroup\$

you say that the FatherID is the record ID of the previous record and that there is only one root record, why would you allow that FatherID to be NULL on creation?

CREATE TABLE [td].[MyTable] (
     [ID] [int] NOT NULL 
    ,[FatherID] [int] NULL
)

Every record has the id of the Father stored in the [FatherID] column. There is only one root element and it is recognized when we found the [FatherID] = 0.

The FatherID field should always have a value no matter what, by allowing null values you are inviting Orphaned records in this funny table. I suggest that you change the Table Schema to not allow Null Values in that column upon record creation.

\$\endgroup\$
3
  • \$\begingroup\$ Unfortunately the Database is not under my control, so I cannot change the schema. Anyway, yes, I strongly agree with you \$\endgroup\$
    – simoneL
    Aug 20 '14 at 21:10
  • \$\begingroup\$ you have created a Create script for the table, and you are inserting into it. \$\endgroup\$
    – Malachi
    Aug 20 '14 at 21:40
  • \$\begingroup\$ To generate the Create schema it's enough to right click on the table desired and there's an action to build the schema. You just need read options. Also be able to insert record inside a database is one thing, be able to modify the schema is another. \$\endgroup\$
    – simoneL
    Aug 20 '14 at 21:44

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.