1. We have added a Gift Upgrades feature that allows you to gift an account upgrade to another member, just in time for the holiday season. You can see the gift option when going to the Account Upgrades screen, or on any user profile screen.
    Dismiss Notice

Stop double-quoting in your SQL!!!!!!!!!!

Discussion in 'Civ5 - SDK / LUA' started by Pazyryk, Aug 9, 2012.

  1. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,584
    I hate to nitpick, but this mis-usage is being propagated by many modders here, even very advanced ones. The weird quoting seems to have evolved from Firaxis' pseudo-SQL XML, and it is not at all correct. It's not just an aesthetic issue. Sure, it works. Until it doesn't. And then you will be scratching your head.

    Here's an example pasted from a tutorial thread:

    Code:
    INSERT INTO "ArtDefine_UnitInfos" ('Type','DamageStates','Formation')
    	SELECT	("ART_DEF_UNIT_WW1_FIGHTER_GERMANY"), "DamageStates", "Formation"
    	FROM "ArtDefine_UnitInfos" WHERE (Type = "ART_DEF_UNIT_WW1_FIGHTER");
    Doesn't matter if you can't follow the SELECT statement (these can make my head spin). The problem is that I can't tell what is a table or column and what is a string literal. The compiler is pretty loose so it gets through it (this time), but only due to blind luck. The correct quoting is like this:

    Code:
    INSERT INTO ArtDefine_UnitInfos (Type,DamageStates,Formation)
    	SELECT	('ART_DEF_UNIT_WW1_FIGHTER_GERMANY'), DamageStates, Formation
    	FROM ArtDefine_UnitInfos WHERE (Type = 'ART_DEF_UNIT_WW1_FIGHTER');
    It looks neater, doesn't it? But that's not the main problem here. Here are the basic rules for quotes.
    1. Use single-quotes (not double quotes!) around string literals. This is the SQL standard.
    2. Don't quote table or column names that are already defined. (You would use single quotes in a CREATE TABLE statement because the table and column names are string literals at that time.)
    3. The purpose of double quotes in SQL is when you want to use a key word as a table or column name (square brackets also do this). For example, I could create a table called "INSERT" and then I would say INSERT INTO "INSERT"... But that would be pretty stupid. Since there is no reason for you to use a reserved keyword for a table or column name, there is no reason for you to ever use a double quote.
    But, you say, "So what, it works fine so leave me alone." Here is where things can go badly wrong: If the string literal just happens to be the same as a column or table name, then your double quoting will screw you over. The SQlite interpreter will interpret it a column or table rather than a string literal.

    It's worth a quick look at the SQlite FAQ if you are going to add much SQL code.
     
  2. Gedemon

    Gedemon Modder Moderator

    Joined:
    Oct 4, 2004
    Messages:
    8,647
    Location:
    France
    I don't remember from which thread I've borrowed my first INSERT INTO ... SELECT ...FROM code, but I wish I would I've known that at the time, now there must be thousands of those double quotes in my SQL files :D

    I'll edit the unit tutorial ASAP at least.

    BTW, the export function of SQLite Manager does put double quotes around table name, any reason for that ?
     
  3. PawelS

    PawelS Ancient Druid

    Joined:
    Dec 11, 2003
    Messages:
    2,803
    Location:
    Poland
    Fortunately I don't quote column names in my code, but I use double quotes for text strings instead of single quotes, so I'll correct that. Thanks for letting us know! (And the FAQ is an interesting read :))
     
  4. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,584
    You can use double quotes (or square brackets) to name a table or column anything, not just keywords but special characters or a name that starts with a digit. I guess you might do this in any code that is generating names that may happen to be illegal in the current SQL or SQlite implementation. This is safe. But unnecessary for us unless you are doing something weird with names. The risky thing is using them for string literals and having that interpreted as table or column names.
     
  5. Thalassicus

    Thalassicus Bytes and Nibblers

    Joined:
    Nov 9, 2005
    Messages:
    11,057
    Location:
    Texas
    For those without formal education in programming, a good guideline is to learn coding practices from reference sites at the top of google searches like w3schools.com/sql or cplusplus.com. Sites lots of people link to usually involve common industry practices (though not always). :)
     
  6. Gedemon

    Gedemon Modder Moderator

    Joined:
    Oct 4, 2004
    Messages:
    8,647
    Location:
    France
    I suppose INDEX a reserved keyword ?

    Because changing my code to the standard is giving me errors now... If so, there is sadly one reason to use double-quote, at least when inserting in the ArtDefine_UnitMemberCombatWeapons table, Index being a column name there.

    You all know Firaxis "special" naming conventions now, so it may be the case with some other tables...

    A relative question: does anyone know a program/script that would allow me to change hundreds of lines of SQL to the standard, taking care of reserved keywords in the process ? :please:
     
  7. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,584
    Well, despite my rant, there isn't any harm in double quoting table or column names. It doesn't do anything (good or bad) if the table or column name is legal. (Single quoting identifiers like column names actually is incorrect, but works for now as explained in edit2 below.)

    It's really the double quotes in your string literals that are dangerous. You're asking the parser to try to read it as a table or column name. Failing that, it seems to fall back and take it as a string literal.

    There are many cases in SQlite where you can do the wrong thing and it still works. For example, if a column is originally defined with integer "affinity", then you can enter data as either '10' or 10. The parser does the work of converting the string to a number, but it's much clearer to just use an integer when you want an integer (this is again a place where Firaxis' pseudo-SQL XML causes some confusion).

    Conversely, when the parser expects a table name or column name, it seems to be OK if these are entered as string literals (single quoted). But really, it helps you understand the language better if you treat these terms (after created in a CREATE TABLE or ALTER TABLE statement) as reserved words. I.e., leave them naked.

    I should say that I'm no expert on SQlite (I get quite confused at complicated SELECT statements). But I built my total conversion mod entirely in SQL rather than XML except for text tags. Here's a bit of it. Looks better with only single-quotes and only where needed, doesn't it?
    Spoiler :


    Edit: To answer your question, INDEX is a reserved keyword. See list here. Keep in mind that SQL (and SQlite) are case-insensitive, so don't try to dodge a keyword that way.

    Edit2: This quote from the link above explains why your code is working despite quote misuse:

     
  8. Aerdan

    Aerdan Chieftain

    Joined:
    Jul 19, 2008
    Messages:
    7
    No. W3Schools is often misleading or outright wrong; W3Fools covers HTML-related problems with W3Schools, but there's no reason to believe they're any better with other technologies.
     
  9. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,584
    Well, it seems that Firaxis uses at least one reserved keyword as a column name:
    Code:
    INSERT INTO ArtDefine_UnitMemberCombatWeapons (UnitMemberType, [COLOR="Red"]Index[/COLOR], SubIndex, ID, VisKillStrengthMin, VisKillStrengthMax, ProjectileSpeed, ProjectileTurnRateMin, ProjectileTurnRateMax, HitEffect, HitEffectScale, HitRadius, ProjectileChildEffectScale, AreaDamageDelay, ContinuousFire, WaitForEffectCompletion, TargetGround, IsDropped, WeaponTypeTag, WeaponTypeSoundOverrideTag)
    VALUES ('ART_DEF_UNIT_MEMBER_CHIMERA', 0, 0, null, 1.0, 1.0, null, null, null, 'ART_DEF_VEFFECT_FIGHTER_MACHINE_GUN_HIT_$(TERRAIN)', 0.25, 15, null, null, 1, null, null, null, 'BULLETHC', 'BULLETHC');
    (Copied from another post.)

    You would need double-quotes [or square brackets] around "Index" to make this command work. I should re-title the post: Stop double-quoting in your SQL (except where you need to)!!!!!!!!!!
     
  10. Gedemon

    Gedemon Modder Moderator

    Joined:
    Oct 4, 2004
    Messages:
    8,647
    Location:
    France
    Yep, that's what I reported in my previous post.

    About the title, just confirm it, and I'll rename it... What about "Do not blindly use double-quotes in your SQL !" and moving it to the tutorial subforum ?
     
  11. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,584
    Sure thing. I keep thinking about writing a long SQL tutorial, but my mod is too close to alpha so I'm just not going to get to it for a while.
     
  12. Aerdan

    Aerdan Chieftain

    Joined:
    Jul 19, 2008
    Messages:
    7
    Eww. I see an SQL injection vulnerability in that query. :(
     
  13. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,584
    I'm assuming that last comment is a joke. If not, please enlighten us. Most of us (including me) are not experts on this topic.
     
  14. Aerdan

    Aerdan Chieftain

    Joined:
    Jul 19, 2008
    Messages:
    7
    No joke. Let me break it out for you:

    Code:
    'ART_DEF_VEFFECT_FIGHTER_MACHINE_GUN_HIT_$(TERRAIN)',
    $(TERRAIN) is a variable substitution, where $TERRAIN could be literally anything. If it's a user-supplied value and if the substitution is done outside of the SQL engine (highly probable), someone could inject their own SQL queries by simply offering a $TERRAIN value such as...

    Code:
    PLAINS',
        0.25,
        15,
        null,
        null,
        1,
        null,
        null,
        null, 
        'BULLETHC',
        'BULLETHC'
    );
    // A malicious query here.
    INSERT INTO ArtDefine_UnitMemberCombatWeapons (
        UnitMemberType,
        Index,
        SubIndex,
        ID,
        VisKillStrengthMin,
        VisKillStrengthMax,
        ProjectileSpeed,
        ProjectileTurnRateMin,
        ProjectileTurnRateMax,
        HitEffect,
        HitEffectScale,
        HitRadius,
        ProjectileChildEffectScale,
        AreaDamageDelay,
        ContinuousFire,
        WaitForEffectCompletion,
        TargetGround,
        IsDropped,
        WeaponTypeTag,
        WeaponTypeSoundOverrideTag
    ) VALUES (
        'ART_DEF_UNIT_MEMBER_CHIMERA',
        0,
        0,
        null,
        1.0,
        1.0,
        null,
        null,
        null,
        'ART_DEF_VEFFECT_FIGHTER_MACHINE_GUN_HIT_
    
    ...and so on.
     
  15. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,584
    OK. That's what I get for posting someone else's code I guess.

    I still don't understand the threat. Am I supposed to be worried about someone hacking my Civ5 game? If it's not obvious, I'm kind of a dummy in these matters.
     
  16. Aerdan

    Aerdan Chieftain

    Joined:
    Jul 19, 2008
    Messages:
    7
    Someone could conceivably write a mod which could break Civ5, at the very least, yes. I'm not sure if mods could break out of Civ5 and affect the rest of your system, though.
     
  17. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,584
    I'm pretty good at breaking the game myself. I do it about 20 times every time I sit down to do a bit of modding.
     
  18. ww2commander

    ww2commander Chieftain

    Joined:
    Aug 23, 2003
    Messages:
    1,243
    Location:
    Australia
    I almost spat my coffee out my nose laughing after reading this!

    Its funny cause its true for me too!:lmao:
     
  19. Horem

    Horem Chieftain

    Joined:
    Apr 1, 2010
    Messages:
    1,718
    Location:
    Wales, UK
    That is taken directly from the civ5coredatabase.db, along with a bunch of others:
    ART_DEF_VEFFECT_ANTIAIR_IMPACT_$(TERRAIN)
    ART_DEF_VEFFECT_CANNON_IMPACT_$(TERRAIN)

    To name but 2.
     
  20. Pazyryk

    Pazyryk Chieftain

    Joined:
    Jun 13, 2008
    Messages:
    3,584
    @Horem,

    I see. It must be an inside job. Like when Gaius Baltar let the Cylons in.
     

Share This Page