Guidance with parsing CREATE sql for Sql Server

[brent-clintel]

When running rake db:migrate against a SQL Server database I get the following stack trace:

NoMethodError: undefined method `[]' for nil:NilClass
./vendor/ruby/2.1.0/gems/schema_plus_core-1.0.0/lib/schema_plus/core/sql_struct.rb:24:in `parse!'
./vendor/ruby/2.1.0/gems/schema_plus_core-1.0.0/lib/schema_plus/core/active_record/connection_adapters/abstract_adapter.rb:48:in `block in visit_TableDefinition'
...

The sql being parsed is:

CREATE TABLE [schema_migrations] ([version] nvarchar(4000) NOT NULL) 

The regex in the SchemaPlus::Core::SqlStruct#parse! method returns nil because of the brackets used around the table name (possibly amongst other things).

I'm happy to try to fix this my self, but I've never delved into this code base before and have no idea what the purpose of this method is.

I don't actually understand why the sql is being parsed in the first place... there's an assemble method that seems to put all the parts back together... Why? Does the SQL need to be parsed at all?

If nothing more, I'm looking for guidance on how to resolve this:

• What is the purpose of this method?
• Why get the parts of the query only to piece it all back together manually?
• What is the correct way to inject a SQL Server specific implementation?

[ronen]

Thanks for taking an interest. Fixing this should be a "simple matter" of fiddling the regex to properly parse the square brackets, and doing something so that the assemble method will close the square brackets.

That said, I imagine that many other things are likely broken for SQL Server -- SQL Server isn't one of the "officially supported" DBMS's. That's mostly because development of the SchemaPlus family happens on OS X/linux with testing on travis-ci, and so Sql Server isn't a convenient option. If you'd like to dive into setting things up to support SQL Server, I'm not opposed. But it wouldn't be a trivial task: you'd need to set up, say, Appveyor to run the test suite on Sql Server while travis runs the others, and ideally also set up some reasonably convenient mechanism for OS X/Linux developers to be able to test while developing. And of course add all the necessary SQL Server-specific stuff to schema_plus_core. If you're interested in doing all that, I'm happy to discuss it further.

And to answer your other questions: The reason that schema_plus/core/active_record/connection_adapters/abstract_adapter.rb#L47 splits up the query only to assemble it again, is that it's setting up a "middleware stack" that other gems can tap into. That is, schema_plus_core splits up the query into component parts, other gems can examine and possibly modify those components, and at the end of the stack execution schema_plus_core reassembles the resulting query. (E.g. schema_plus/foreign_keys/middleware/sql.rb#L13 adds some terms to the body.) The underlying idea--for all of schema_plus_core--is to put all the ugly hacky dbms-specific and rails-version-specific stuff in schema_plus_core, providing a reasonably clean, simple, and stable interface for other gems to use.

[brent-clintel]

Thanks for your response @ronen. The reasoning makes sense now that you mention it.

I've determined that the following works (at least for the specific error I was getting).

There have been three changes made:

• The END_BRACKET_MAPPING provides mappings for the end quote
• The regex in the parse! method explicitly checks for [ and ]
• The assemble method uses the END_BRACKET_MAPPING to determine the correct end quote to use

module SchemaPlus
  module Core
    module SqlStruct
      IndexComponents = KeyStruct[:name, :type, :columns, :options, :algorithm, :using]

      class Table < KeyStruct[:command, :name, :body, :options, :quotechar, :inheritance]

        INHERITANCE_REGEX = %r{ \s* (?<inheritance>INHERITS \s* \( [^)]* \)) }mxi
        END_BRACKET_MAPPING = {?' => ?', ?" => ?", ?` => ?`, ?[ => ?]}

        def parse!(sql)
          m = sql.strip.match %r{
          \A
          (?<command>.*\bTABLE\b) \s*
            ((?<quote>['"`])(?<name>\S+)\k<quote>|(?<quote>[\[])(?<name>\S+)\]) \s*
            \( \s*
            (?<body>.*) \s*
            \) \s*
            # can't use optional ? for inheritance because it would be greedily grabbed into body;
            # ideally this would use an actual parser rather than regex
            #{INHERITANCE_REGEX if sql.match INHERITANCE_REGEX}
            (?<options> \S.*)?
          \Z
          }mxi
          self.command = m[:command]
          self.quotechar = m[:quote]
          self.name = m[:name]
          self.body = m[:body]
          self.options = m[:options]
          self.inheritance = m[:inheritance] rescue nil
        end
        def assemble
          ["#{command} #{quotechar}#{name}#{END_BRACKET_MAPPING[quotechar]} (#{body})", inheritance, options].reject(&:blank?).join(" ")
        end
      end
    end
  end
end

Am I right by assuming that a pull request with this change wouldn't get accept without support for running tests in a SQL Server environment? If it will get accepted I'll submit the commit.

Unfortunately I'm not confident that I can commit to implementing a solution for all SQL Server issues, however, I would be keen to run the test suit to see what issues there are. How do I get the tests running for schema_plus_core?

The other issue I see with implementing a complete SQL Server solution is that each of the schema_plus extension gems would require a similar separate testing environment.

[ronen]

@brent-clintel your fix does look good. But you're right, without a test suite for SQL Server I wouldn't want to accept a PR for it.

For info on running the test suite, see the Development & Testing section of the schema_plus_core README. The quick answer is you run $ bundle exec schema_dev rspec which sets things up to run the specs on any/all combinations of db, rails, and ruby version that the testing is configured for. Of course it won't set things up for SQL Server -- but you can run $ bundle exec schema_dev -n rspec and see the detailed commands it would run, and pattern match on that.

...and that gives the hint to your question about setting up the testing environment for each gem in the schema_plus family: schema_dev takes care of setting everything up, based on a file schema_dev.yml in each gem's root. So the way to go about adding SQL Server support would be to add it to schema_dev.