6
\$\begingroup\$

Business logic

It would take us too far to explain the whole business case behind this code, so I'll try to be as succinct as possible.

  • There are Persons,
  • and Persons can have Profiles,
  • and the combination of those two is a PersonProfile.
  • There is also a table called ProfilesToReplace, which contains Profiles that in the future will be replaced by updated ones.
    • For compatibility sake these ProfilesToReplace are not linked to the actual Person, but to a "dummy" Person.

Over time, entries in ProfilesToReplace will be deleted (once an outdated Profile has been replaced by a proper one), which means the related Profiles need to be removed. But before that can happen, we need to remove the PersonProfiles that link to these outdated Profiles and remove these first. We also need to remove the "dummy" Persons once they have no related entries in the PersonProfiles table.

This is done via a regularly running job, which is a .NET C# 4.6.2 command line application (but it uses the new csproj format).

The problem

There is however a snag. To know which Person is a "dummy" person, we need to look at that Person's IDF: in the case of a "dummy" Person, it is the same as the IDF of its related "real" Person, except for a single digit. We do have a list of "real" IDFs, but that is in another database, and it is not possible to connect our database to that other database.

The good news: we can retrieve a list of these "valid" IDFs in our application. The bad news: this involves tens of thousands of IDFs, and thus we're confronted with the limits of SQL Server parameters. Moreover, we're working with EntityFrameworkCore and I haven't found a way to pass a list of data via .FromSql(). Also, retrieving all data and filtering in code is problematic as well, since PersonProfiles contains more than a million records.

How I solved it

However, I found a way around these issues, allowing me to do the filtering in a query: I pass the list of IDFs as an XML string and let the query untangle it into a list of IDFs that can be used in a WHERE clause:

SELECT DISTINCT [Id]

FROM [authorization].[PersonProfile]

WHERE 1 = 1
    AND [ProfileId] NOT IN (
        SELECT [ProfileId]
        FROM [authorization].[ProfilesToReplace]
    )
    AND [PersonIdf] IN (
        SELECT PersonIdf
        FROM (
            SELECT PersonIdf = XTbl.value('(PersonIdf)[1]', 'bigint')
            FROM @PersonIdfs.nodes('/root') AS XD(XTbl)
        ) AS XmlToData
    )

And this is how I use that SQL and pass it the necessary parameter:

public async Task<ICollection<int?>> Execute(List<long> dummyPersonIdfs)
{
    var getDeletablePersonProfilesQuery = QueryRetriever.GetQuery("GetDeletablePersonProfiles.sql");
    var personIdfsAsXml = new XDocument(
            new XElement("root", dummyPersonIdfs.Select(x => new XElement("PersonIdf", x))))
        .ToString();
    var personIdfs = new SqlParameter("@PersonIdfs", SqlDbType.Xml) { Value = personIdfsAsXml };
    var deletablePersonProfileIds = (await _dbContext.DeletablePersonProfiles
            .FromSql(getDeletablePersonProfilesQuery, personIdfs).ToListAsync())
        .Select(x => x.Id).ToList();

    if (deletablePersonProfileIds.Any())
    {
        return await _personsRemover.Execute(deletablePersonProfileIds);
    }

    return new List<int?>();
}

QueryRetriever is simply a class that retrieves the above SQL query from an embedded .sql file:

internal static class QueryRetriever
{
    public static string GetQuery(string scriptName)
    {
        var namespacePrefix = "Badger.Service.BadgeCleaning.";
        var resourceName = namespacePrefix + scriptName;
        var assembly = Assembly.GetExecutingAssembly();

        using (var stream = assembly.GetManifestResourceStream(resourceName))
        {
            if (stream == null)
            {
                throw new Exception($"Missing SQL script: {resourceName}");
            }

            using (var streamReader = new StreamReader(stream))
            {
                return streamReader.ReadToEnd();
            }
        }
    }
}

So all's well?

While I am happy with the code as is, I'm wondering whether I've made it more complicated than needs to be. Am I overlooking a simpler solution?

I'm also somewhat unsure of how I construct the XML string: is there perhaps a more efficient way?

And is the way I deal with the XML string in my SQL query the best way? Note that I cannot use STRING_SPLIT, because the application isn't currently running on SQL Server 2016 (or any more recent product).

\$\endgroup\$
1
\$\begingroup\$

If you weren't using EntityFrameworkCore, my suggestion would look something like this:

  1. Get the list of values from database 1
  2. Use SqlBulkCopy to insert a bunch of them into some table on database 2
  3. Join to that table in your "GetDeletablePersonProfiles" query.

Unfortunately, based on some cursory googling, it looks like EntityFrameworkCore doesn't support bulk operations (this extension does, however). I'll admit - I don't know a ton about EFC, but based on what you're doing I assume the following is possible:

  1. Get the list of values from database 1 (you're already doing this)
  2. In the code you execute, dump the values from the XML into a table (better performance than joining to the XML)
  3. Use that table in your query

Specifically, the main change I would suggest is taking this:

AND [PersonIdf] IN (
    SELECT PersonIdf
    FROM (
        SELECT PersonIdf = XTbl.value('(PersonIdf)[1]', 'bigint')
        FROM @PersonIdfs.nodes('/root') AS XD(XTbl)
    ) AS XmlToData
)

And turn it into this (you'll notice that I've also transformed both of your EXISTS into JOINs; they're easier to read and accomplish the same task.

SELECT PersonIdf = XTbl.value('(PersonIdf)[1]', 'bigint')
  INTO #MyFunTempTable
  FROM @PersonIdfs.nodes('/root') AS XD(XTbl);

SELECT DISTINCT PersonProfile.[Id]
  FROM [authorization].[PersonProfile]
    INNER JOIN #MyFunTempTable PersonIdfList
      ON PersonProfile.PersonIdf = PersonIdfList.PersonIdf
    LEFT OUTER JOIN [authorization].[ProfilesToReplace]
      ON PersonProfile.ProfileId = ProfilesToReplace.ProfileId
  WHERE ProfilesToReplace.ProfileId IS NULL

It would also be good to make this a stored procedure, so that you don't have to worry about GetDeletablePersonProfiles.sql being replaced by a malicious actor.

You could also split it into two stages, using ExecuteSqlCommand to insert the values into a table (can't be a temp table anymore) and then you should really just need LINQ to get the values back.

You can also get an ADO.NET DbConnection object from the database context using _dbContext.Database.GetDbConnection(), and if you know that this is a SQL Server connection I've heard that you can cast that to a SqlConnection object, at which point you have access to the SqlBulkCopy method. This is questionable at best, but it might provide clean enough/performant enough code to be worthwhile. I'm definitely not endorsing this approach, but I'll put it out there.

\$\endgroup\$

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.