5
\$\begingroup\$

Over on EL&U Meta, someone raised the question of how many net migrations there are to/from our sister site, ELL.

The following SEDE query calculates the net migrations by quarter (elu2ell - ell2elu). You can play with the query in SEDE here.

-- migration prefix length
DECLARE @mhpl INT, @mapl INT, @mhid INT, @maid INT;

SELECT @mhid = Id from PostHistoryTypes where Name = N'Post Migrated Here'
SELECT @maid = Id from PostHistoryTypes where Name = N'Post Migrated Away'

-- Create copy of PostHistory table with
-- all the posts migrated here from other sites
SELECT * INTO #ph FROM posthistory WHERE PostHistoryTypeId in (@mhid, @maid)

-- Add "migration direction" column 
ALTER TABLE #ph ADD mDir CHAR(1) NULL;
UPDATE #ph SET mDir = '<' WHERE PostHistoryTypeId=@mhid
UPDATE #ph SET mDir = '>' WHERE PostHistoryTypeId=@maid

-- Add column for site where question was migrated from
ALTER TABLE #ph ADD mSite varchar(255) NULL;

-- Parse migration source site from comment field
-- and put into column
SET @mhpl = LEN(N'from http://')
SET @mapl = LEN(N'to http://'  )
UPDATE #ph SET mSite = 
  replace( 
    CASE WHEN mDir = N'<' THEN 
      substring(
                 COMMENT, @mhpl, 
                 charindex('.',COMMENT,@mhpl) - @mhpl
               )
    ELSE 
      substring(
                 COMMENT, @mapl, 
                 charindex('.',COMMENT,@mapl) - @mapl
               )
    END,
    N'/', ''
  )


-- Add calendar quarter column column for site where question was migrated from
ALTER TABLE #ph ADD CalendarQuarter CHAR( 7 ) NULL; -- LEN(N'FYXXQYY')
UPDATE #ph SET CalendarQuarter = N'FY' + 
               RIGHT(
                      CAST( YEAR(CreationDate) AS CHAR(4) ), 
                      2 
               ) +
               N'Q' + 
               CAST( CEILING(
                             CAST(
                                  MONTH(CreationDate) AS DECIMAL(4,2)
                                  ) / 3 
                             ) AS char(1)
               )

SELECT 
  CalendarQuarter, 
  SUM(CASE WHEN mDir='>' THEN 1 ELSE  0 END) as Departing,
  SUM(CASE WHEN mDir='<' THEN 1 ELSE  0 END) as Arriving,
  SUM(CASE WHEN mDir='<' THEN 1 ELSE -1 END) as NetDepatures
FROM #ph
WHERE mSite='ell'
GROUP BY CalendarQuarter
ORDER BY CalendarQuarter DESC

I've used SQL here and there in my professional career, but I'm far from an expert. I'd like commentary on the overall query, structure, organization, style. Particular areas of concern:

  • The duplication of the charindex('.',COMMENT,@mhpl) - @mhpl logic in the migration-site parsing UPDATE statement bugs me,
  • as does the general verbosity and complexity of the CalendarQuarter-formatting code.
  • You'll also see I use vertical indentation for most deeply-nested function calls. Is this normal or useful practice for complicated SQL queries? If not, how do you help the reader visually parse deeply-nested parens, without wasting so much real estate?

Beyond that, performance optimizations are nice, but it's a small enough dataset that it doesn't matter that much.

A final issue is I worry that the way I've structured the temp table will limit more general analysis to other (non-ELL) sites and over different time horizons, in particular by adding an explicit CalendarQuarter column.

I'd like to do that dynamically for the sake of the final query (where I do want quarterly figures), but I found it difficult to both dynamically calculate that column as well GROUP BY it.

So, if there's a way to structure the final table that makes future, more general analytics easier, that would be useful feedback.


I am also suspicious of the PostHistory table and whether I can rely on PostHistoryTypeId in [35, 36] to reliably tell me that a question was migrated. But this is CR, not SO, so let's assume the logic is correct, and focus on the code as code.

In re: future extensions, I am aware that meta sites in general will not be categorized properly, as they all collapse down to meta before Great HTTPs Migration, and are counted among the normal site migrations after it (i.e. english.meta.stackexchange and english.stackexchange both map to english now). But meta-migrations are rare enough for my purposes that I can ignore them. Nor am I concerned with hypothetical migrations to non-*.stackexchange.com sites (like the original trilogy). That won't happen for EL&U.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks for this great question - I hope you get some good reviews, and I hope to see more of your contributions here in future! \$\endgroup\$ – Toby Speight Oct 25 '18 at 8:22
  • \$\begingroup\$ @TobySpeight Thanks! I'm not sure what the general cadence of CR is, but on the other sites I'm active in, a question not having gotten any feedback in a day is a good predictor that it never will. I'm happy to leave this up though, maybe one day someone will run across it. In the meantime, do you feel there is anything I can do to improve it, to increase its appeal and attract attention? Of course, if you feel you'd like to give a shot at a review yourself, that would also be very welcome. \$\endgroup\$ – Dan Bron Oct 25 '18 at 17:06
  • \$\begingroup\$ CR tends to move at a more considered pace than most, so don't panic. I don't see anything you need to do to improve this question; it may require more patience than beginner-level code in the more popular languages. If I were more than a mere dabbler in SQL, I'd attempt a review myself, but as it is I can offer nothing of worth; sorry! \$\endgroup\$ – Toby Speight Oct 29 '18 at 8:52
1
\$\begingroup\$

I'd like commentary on the overall query, structure, organization, style.

The query is consistent for style. Its structure and organization seems to match the functional decomposition you did to achieve the desired result.

While the use of a temporary table to shape, reduce and transform your data isn't uncommon SQL Server does offer an alternative called Common Table Expression and if you don't have special needs (like indexes for example) I would prefer one or more common table expressions (shorthand CTE).

For reference, here is the Execution plan:

execution plan showing updates and spooling

You could still have each functional step in your query be represented as an CTE and join them later. As the columns that you add are a projection over the same row each time there is not really a need to separate those in different statement. So the temptable #ph can be replaced with a CTE where I think you can still capture the functional steps:

-- migration prefix length
DECLARE @mhpl INT, @mapl INT, @mhid INT, @maid INT;

SET @mhpl = LEN(N'from http://')
SET @mapl = LEN(N'to http://'  )

SELECT @mhid = Id from PostHistoryTypes where Name = N'Post Migrated Here'
SELECT @maid = Id from PostHistoryTypes where Name = N'Post Migrated Away'

-- Create copy of PostHistory table with
-- all the posts migrated here from other sites
;WITH ph as (
  SELECT *
        -- Add "migration direction" column 
       , (CASE PostHistoryTypeId 
          WHEN @mhid THEN '<'
          WHEN @maid THEN '>'
          END
         ) mDir
         -- Add column for site where question was migrated from
         -- Parse migration source site from comment field
         -- and put into column
       , replace( 
            CASE WHEN PostHistoryTypeId = @mhid THEN 
              substring(
                         COMMENT, @mhpl, 
                         charindex('.',COMMENT,@mhpl) - @mhpl
                       )
            ELSE 
              substring(
                         COMMENT, @mapl, 
                         charindex('.',COMMENT,@mapl) - @mapl
                       )
            END,
            N'/', ''
          ) mSite
       -- Add calendar quarter column column for site where question was migrated from
      , N'FY' + 
         RIGHT(
                CAST( YEAR(CreationDate) AS CHAR(4) ), 
                2 
         ) +
         N'Q' + 
         CAST( CEILING(
                       CAST(
                            MONTH(CreationDate) AS DECIMAL(4,2)
                            ) / 3 
                       ) AS char(1)
         ) CalendarQuarter
  FROM posthistory 
  WHERE PostHistoryTypeId in (@mhid, @maid)
)

Here is the Execution plan of the whole query:

single execution stream

With that tackled lets move on to your raised concerns:

Particular areas of concern:

  • The duplication of the charindex('.',COMMENT,@mhpl) - @mhpl logic in the migration-site parsing UPDATE statement bugs me,
  • as does the general verbosity and complexity of the CalendarQuarter-formatting code.
  • You'll also see I use vertical indentation for most deeply-nested function calls. Is this normal or useful practice for complicated SQL queries? If not, how do you help the reader visually parse deeply-nested parens, without wasting so much real estate?

If you had your own SQL Server instance you can reduce duplication to introduce User-defined functions but unfortunately that is not allowed on SEDE. We lack the permission to create those. So it is not easy to prevent duplication in this setting.

Looking at the content of the comment field we find that it contains either from https://somesite.stackexchange.com or to https://somesite.stackexchange.com with the added complexity that older records still have http:// in it. You're interested in the somesite part.

Except the charindex function there also exists patindex. It let's you search for a pattern and I used it to search for %http%:// but I found that I couldn't handle http or https nicely so in the end I kept the patindex for demo purposes but charindex would have worked as well. What I basically did is: search for :\\ and search for a single dot, substract those and you'll have the first part of the domainname. That eliminates the need for the CASE/THEN/ELSE construction.

Applying that will give you this expression:

DECLARE @http NVARCHAR(9), @httplen INT

SET @http = '%://%';
SET @httplen = LEN(@http) - 2; -- don't count the %

-- in the query
SUBSTRING(
   COMMENT, 
   PATINDEX(@http, comment) + @httplen, 
   CHARINDEX('.', COMMENT) - (PATINDEX(@http, comment) + @httplen)
 ) mSite

For the CalendarQuarter formatting I would prefer to leave that to the final stage. So in the CTE we only need to supply columns for the year and the quarter. SQL Server has a DATEPART function that does allow to specify quarter as a datepart. To reduce the Year to the part without the century I apply a modulo of 100. That makes that in the CTE you no longer have to fiddle with varchars and casts.

Here is how those functions pan out for Year and Quarter:

, DATEPART(yy, CreationDate) % 100 fy
, DATEPART(qq, CreationDate) q

In the final query you can now combine those however you like, for example:

CONCAT('FY', fy, 'Q', q) as CalendarQuarter,

which leaves the conversion of values to the correct type to SQL Server.

Do note that you can now add extra columns to the CTE for different time/date series and only adapt the group by and projection to show the result.

For completeness here is the complete query:

DECLARE @http NVARCHAR(5), @httplen INT, @mhid INT, @maid INT;

SET @http = '%://%';
SET @httplen = LEN(@http) - 2; -- don't count the %

SELECT @mhid = Id from PostHistoryTypes where Name = N'Post Migrated Here'
SELECT @maid = Id from PostHistoryTypes where Name = N'Post Migrated Away'

;WITH ph as (
  SELECT *
       , (
          CASE PostHistoryTypeId 
          WHEN @mhid THEN '<'
          WHEN @maid THEN '>'
          END
         ) mDir
       , SUBSTRING(
           COMMENT, 
           PATINDEX(@http, comment) + @httplen, 
           CHARINDEX('.', COMMENT) - (PATINDEX(@http, comment) + @httplen)
         ) mSite
       , DATEPART(yy, CreationDate) % 100 fy
       , DATEPART(qq, CreationDate) q
  FROM posthistory 
  WHERE PostHistoryTypeId in (@mhid, @maid)
)


SELECT 
  CONCAT('FY', fy, 'Q', q) as CalendarQuarter,
  SUM(CASE WHEN mDir='>' THEN 1 ELSE  0 END) as Departing,
  SUM(CASE WHEN mDir='<' THEN 1 ELSE  0 END) as Arriving,
  SUM(CASE WHEN mDir='>' THEN 1 ELSE -1 END) as NetDepartures
FROM ph
WHERE mSite='ell'
GROUP BY fy,
         q
ORDER BY fy DESC,
          q DESC

A final word about indenting. I have seen enough variations over the years and I'm not religious about it. If anything, you could stick to indent one stop, being it 2 or 4 spaces, instead of aligning vertical. To apply that to something we saw in the first snippet will give you this:

replace( 
  CASE WHEN PostHistoryTypeId = @mhid THEN 
    substring(
      COMMENT, 
      @mhpl, 
      charindex('.', COMMENT, @mhpl) - @mhpl
    )
  ELSE 
    substring(
      COMMENT, 
      @mapl, 
      charindex('.', COMMENT, @mapl) - @mapl
    )
  END,
  N'/', 
  ''
) mSite

That is a bit more compact without compromising too much of readability.

\$\endgroup\$
1
  • \$\begingroup\$ Wow, this is the kind of both holistic and detailed guidance and expertise I come here for! Thank you. \$\endgroup\$ – Dan Bron Aug 20 '19 at 21:50

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.