This topic contains 19 replies, has 3 voices, and was last updated by  Eliot Muir 3 years ago.

SQL Server DateTime day and month swapped

  • Hi, I don’t know if this issue can be handled in Iguana, but here it goes:
    When I write DateTime values to an SQL Server database, and the user has set a language other than US English, the day and month values are swapped.

    Let’s say I create a little dummy database:

    
    create database Christmas;
    create table Christmas.dbo.WorkingDays ( theday DATETIME );
    CREATE LOGIN santa WITH PASSWORD = 'cl@us4711', DEFAULT_DATABASE = Christmas, CHECK_POLICY= OFF;
    
    USE Christmas;
    CREATE USER santa FOR LOGIN santa;
    EXEC sp_addrolemember 'db_datareader', 'santa'
    EXEC sp_addrolemember 'db_datawriter', 'santa'
    GO
    

    And my Lua code looks like this:

    function main()
       local conn = db.connect({api = db.SQL_SERVER, name = "Christmas", user = "santa", password = "cl@us4711",
       use_unicode=true,
       live = true})
       local message = db.tables{vmd = 'Christmas.vmd', name='Message'}
       local wday = message.WorkingDays[1]
       wday.theday = '2014-12-26 12:34:56'
       conn:merge{data = message, live = true}
    end
    

    All works fine. But if I set the language of the user “santa” to something other than English, e.g. alter login santa with DEFAULT_LANGUAGE = German; (or in the Management Studio under “Security / Logins / santa / Properties / General / Default language”), I get a date conversion error. (If the day is between 1 and 12, there is no error, but the date is wrong.)

    This is a known issue in SQL Server date conversion, see e.g. here or here.

    Of course, the obvious workaround is to set the user’s language to English, but is there a way to make the Iguana database connection more robust concerning the language settings?

    You could grab the Lua implementation of db.merge and give it some smarts to detect different languages. If the changes are good we could start putting that module in the Iguana App repository so the code can be maintained.

    It’s bit tricky for us to test all the different European languages since we’re not experts in all the different languages.

    This may be a dumb question, but: Where can I grab the Lua implementation of db.merge? It would be interesting to have a look at it.

    I don’t think you need to test all languages… maybe it’s as simple as replacing in the SQL code “2014-12-26 12:34:56” by “2014-12-26T12:34:56”, with the “T” in the middle the date format is not ambiguous any more.

    http://code.interfaceware.com/code is something we tried in the past as a better way to share code with our users – but the new git hub repository is better – but as of today that’s the most up to date version of the code.

    BTW – Robin have you seen:
    http://help.interfaceware.com/forums/topic/tired-of-editing-vmd-files-for-table-schema

    Feedback on that is welcome.

    So, the code is this one? http://code.interfaceware.com/code?file=db_merge.lua&format=view

    In lines 43-45, you just call “tostring” on the datetime value and hope the database will parse it correctly…
    I guess for SQL Server it would be better to use something like

       R[#R+1] = 'convert(varchar,'
       R[#R+1] = '\''
       R[#R+1] = tostring(N)
       R[#R+1] = '\''
       R[#R+1] = ', 121)'
    

    Perhaps add a special function, analogous to OracleDateTimeCvt.
    I think this would fix our issue.

    About the new database schema stuff, I just saw it today, looks like a good idea, but I still have to look at it more thoroughly.

    Aaarrgh, of course “convert(datetime”, not “convert(varchar”, sorry!

    Ever forked a GIT repo?

    I beg your pardon?

    I guess not 🙂

    It’s that I don’t understand what this has to do with date parsing…

    Well – if you fork our community repo, make a quick channel that has the merge functionality with these language tweaks in it. Use the channel manager to export the channel into your own repository.

    Then you can issue what is called a ‘pull request’. We’d be able to pull your tweaked version of the code into the main repo and from there we could start maintaining it, adding unit tests and so on.

    It would make it possible for other people in the user community to contribute other tweaks to polish it up into a more useful resource.

    Git Hub makes it a relatively easy process to do.

    Ooh, this sounds like fun, I’d like to do that!
    Is there any tutorial which could teach me how to do it?
    I have some basic knowledge of Git (although, unfortunately, too little practice), so the forking and pull request I might accomplish. But:
    Where is the community repo? Is it https://github.com/interfaceware/iguana-web-apps? If so, where is db_merge.lua in there?
    I have a feeling I’m missing a decisive piece of information here… Sorry for that!

    The merge code isn’t in the repo yet – it would need to be added.

    Here’s the start of a unfinished tutorial explaining how to get started with doing a fork:

    http://help.interfaceware.com/kb/cloning-the-iguana-apps-repo-using-github

    If you follow it along and get to the point that you are ready to come to grips with the GIT command line I’ll be happy to flesh it out with some more detail on the core commands to use. It’s a useful skill to know GIT – I think it’s really become the clear winner of the new wave of distributed source control systems.

    Wow, thanks for putting this together, I’ll give it a try! Yesterday I tried it with the command-line git, but I’m behind a firewall, and the proxy wouldn’t let me through.

    Coming back to the original topic: If I want to patch my local db.merge function to fix the bug, can I use the db_merge from code.interfaceware.com? (Is it up to date?) Or is custom_merge.lua better? Or do I have to wait for you to upload the newest version to GitHub?

    Hello again,

    with custom_merge.lua it works fine, after adding a function

    local function SqlServerDateTime(Node, Buffer)
       Buffer:write('CONVERT(DateTime, \'', Node, '\', 121)')
    end
    

    and changing line 168 to

       [db.SQL_SERVER]  = {Merge = genericMergeSQL, Batch = batchBeginEnd, AddValue = {datetime = SqlServerDateTime}},
    

    Do you think you could consider fixing this in the next version of Iguana?

    It would be very risky to make this part of the GA release of Iguana for the reasons explained here:

    http://help.interfaceware.com/forums/reply/10642

    However it would be great to get a version of this into the Iguana repo, see how it grows and if it gets some legs. I’ll be happy to help you through the process of doing that with figuring out how to use GIT and GitHub.

    I guess the module probably should be named slightly differently to make it clear (at least at this stage that it only supports certain locales and certain databases and it can referenced back to this thread.

    Sure, you can let the code mature for a while, but in the long run I think you should fix the bug. Just passing a datetime string to SQL Server is always prone to misinterpretations, so you should always either pass the string as yyyy-mm-ddThh:mm:ss (with the ‘T’ in the middle, it is not locale-dependent) or explicitly call CONVERT with a fixed datetime style (here, 120 or 121).

    For me, it is fine if you can confirm that custom_merge.lua is up to date and can readily replace the buggy default version of db.merge. It’s just that we had this nasty bug with customers who had a French/Italian/whatever version of SQL Server, and others might fall in this trap.

    Alrighty … I guess we can dig a little more into the code. I think it’s complicated by the fact that the internal routine is complicated more than one might think with parameter binding etc. ah well. Let’s move the Lua version along a little and see where it gets to.

You must be logged in to reply to this topic.