[freeside-devel] MSSQL

Sat Oct 7 03:49:26 PDT 2017

IT'S ALIIIIIIIIVE!

I can now log in, get to the main billing screen, and navigate around a
bit.

I do believe that there are a number of DBMS portability issues that
this exercise brought to light which Freeside will benefit from
long-term since I believe a good number of them can be fixed in a
universal way.  Notes so far, in case anyone is interested:

---
01. Just to start with, I've been working against FS 3.9x on the
assumption that because 3.x still more or less officially supports MySQL
while 4.x apparently has known regressions in this area, that there
would be less to do before I saw results (assuming as much code as
possible was being shared between My and Pg support).  I now suspect
I'll discover once I start poking at 4.x that it made little difference
(when I started, I thought that DBIx::DBSchema and its drivers were
doing more to abstract stuff away than they are, and found a lot more
"if Pg elsif mysql" branching in Freeside proper than I expected).

---
02. So far, most of the problems I'm running into with static queries
defined outside of FS::Record have to do with liberal use of "JOIN
USING", which is less portable than I'm sure we'd all wish, and which
sadly MSSQL doesn't support.  Even though I like it better because it's
concise and I find it more readable, I'm going to need to slog through
every module that uses this convention and change all of these to "JOIN
ON".

---
03. Inside FS::Record, in order to work around an escaping issue (MSSQL
interprets backslash followed by newline as a line extension, and throws
out both bytes...this causes problems with slurping up LaTeX), I've had
to change INSERTs (insert() and _h_statement()) and UPDATEs (replace())
so that they use parameterized queries instead.  This meant changing
_quote() to not call DBI::quote() and just return the untouched string,
and also to return undef instead of 'NULL'.  This in turn meant that I
had to modify any subroutine that called _quote() so that it used
parameterized queries as well (e.g., delete()).  This all works fine in
my "lab", and I have a hard time believing that this will break anything
for other databases, but I haven't tested other databases against this
change yet.

---
04. There are apparently some Perl DBDs that do not support having more
than one open/active statement handle if AutoCommit is disabled (you
can't have multiple statements prepared...you have to prepare, then
execute, then prepare, then execute, etc. serially).  Turns out
DBD::Sybase is one of them.  There were a couple of spots in FS::Record
where this happens (sometimes both the regular table and the history
table(s) have their statements prepared one after the other), so I had
to shuffle things around a little to avoid this.

---
05. One of the more baffling things I ran into in the Freeside code
itself is the use of '' to represent empty strings in a query if using
Pg, but using "" to represent them for any engine other than Pg...which
up until now I guess meant MySQL.  Why this was even done for MySQL is
the part that confuses me.  MySQL for some strange reason does support
both " and the more standard ' as string delimiters, while virtually no
other SQL engine I can think of supports or uses " for string literals
(I think maybe MSSQL can be goaded into doing so, but why...).  And
since the logic in essence was /if Pg then '' else ""/, instead of the
other way around, /if mysql then "" else ''/, this caused problems.

I just got rid of the double-quotes altogether. :-P

---
06. MSSQL won't index variable-length columns (TEXT, BLOB, VARCHAR(max),
etc.) either, so for now I'm applying the same MySQL "kludge" for the
"statustext" columns to MSSQL as well.  (I'm guessing there is a reason
those columns were being indexed to begin with.  Is this likely to
affect performance much?  MySQL Freeside users -- assuming there are
many of them left -- have seemingly been living with this "kludge" for
some time now, right?)

Along the same lines, the "path" column in the "access_user_page_pref"
table was being indexed but was also of the TEXT datatype.  I changed
this to VARCHAR of length 2*$char_d...I assume this was a simple
oversight and/or a column that was added much later, since this wouldn't
have worked with MySQL, and other tables with a column named "path" are
already VARCHAR(2*$char_d).

---
07. There are some column names ("path", for one) in the schema that are
reserved keywords in MSSQL.  It's not reasonable to have to comb through
and rename tables and columns whenever support for a new DBMS is brought
on-line, nor to ban certain names and have the ban-list size increase as
time goes on, so I think the right answer here is to indiscriminately
quote all table and column identifiers (which should "just work" with
Postgres out-of-the-box), which would require MySQL users to run in
ANSI_QUOTES mode and MSSQL users to run with QUOTED_IDENTIFIERS set. 
(DBD::Sybase lets you do this at connect time with a special database
handle attribute; not sure about MySQL, which may instead require
something like /if mysql then $dbh->do("SET SQL_MODE='ANSI_QUOTES'")/
immediately after connection.)

If you put MySQL into full-on ANSI mode instead of just ANSI_QUOTES,
then you can even simplify FS::Record::concat_sql() by using || for both
Pg and My.  (Unfortunately, MSSQL only supports + for string
concatenation, so this subroutine will still need to exist.)

---
08. Near the beginning, FS::Record::insert() assigns unique values to
all unique index columns...which by definition includes PKs that are
already set to auto-populate and auto-increment.  So the foreach loop at
the top needs to skip over the primary key column.

Similarly, replace() tries to UPDATE a row by including basically all
columns in the UPDATE, which includes SETting primary_key = some_value. 
In my fork, within replace() I'm now taking the result from
real_fields() and removing the primary key from it first, so it is not
included in the UPDATE statement.

I'm not sure how any of the other supported databases were working while
these issues were in place.  Perhaps at least in the second instance,
some DBs might allow you to set a SERIAL primary key column if you are
"setting" it to the same value it already holds?  MSSQL will definitely
throw an error if you try to include a (what is effectively) SERIAL
column in the SET clause of an UPDATE command, even if you end up not
actually changing the stored value.

---
09. I haven't figured out what to do about "table mutexes"
(FS::Record::lock_table() used by FS::svc_*) yet on MSSQL.  For now I'm
faking it and just having lock_table() return without doing anything.

---
10. RT is going to be a problem.  I was hoping, again, to postpone work
on that as much as possible, but "RT_ENABLED = 0" is definitely not
ready for primetime (I understand the Makefile value is substituted in
during install, but even accounting for that it seems there are many
places in the code that outright assume RT is installed and where that
constant is not present).  Fortunately, RT experimentally supports
SQLite, so although I still installed it from the Freeside tarball, I
configured it to store in a SQLite file for now, and then told Freeside
(by editing the database manually, since the configuration screen
wouldn't work with RT in a broken state) that it was an external RT
instance, and that seemed to satisfy it.

Apparently there have been past attempts at porting RT to MSSQL/Sybase,
but those were back in the early RT 3.x days, so I'll probably not be
able to take advantage of those.  I'm not so much concerned about where
our tickets are stored, so if we ever take MSSQL Freeside into
production, we'll likely have RT pointed at a local MySQL db just to
begin with, and worry about moving those to the main database servers
later (if ever).

That's all I can think of for now.

-- 
Nathan Anderson
First Step Internet, LLC
nathana at fsr.com