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

[Pazyryk]

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:

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:

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.

 

[Gedemon]

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

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 ?

 

[PawelS]

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 )

 

[Pazyryk]

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

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.

 

[Thalassicus]

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).

 

[Gedemon]

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.

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 ?

 

[Pazyryk]

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:

For resilience when confronted with historical SQL statements, SQLite will sometimes bend the quoting rules above:

• If a keyword in single quotes (ex: 'key' or 'glob') is used in a context where an identifier is allowed but where a string literal is not allowed, then the token is understood to be an identifier instead of a string literal. [Paz: this is why your single quoted column names are working even though they shouldn't use any quotes]
  
  
• If a keyword in double quotes (ex: "key" or "glob") is used in a context where it cannot be resolved to an identifier but where a string literal is allowed, then the token is understood to be a string literal instead of an identifier. [Paz: this is why your double quoted string literals are working]

Programmers are cautioned not to use the two exceptions described in the previous bullets. We emphasize that they exist only so that old and ill-formed SQL statements will run correctly. Future versions of SQLite might change to raise errors instead of accepting the malformed statements covered by the exceptions above.

 

[Aerdan]

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

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.

 

[Pazyryk]

Well, it seems that Firaxis uses at least one reserved keyword as a column name:

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)!!!!!!!!!!

 

[Gedemon]

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 ?

 

[Pazyryk]

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.

 

[Aerdan]

Eww. I see an SQL injection vulnerability in that query.

 

[Pazyryk]

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.

 

[Aerdan]

No joke. Let me break it out for you:

'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...

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.

 

[Pazyryk]

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.

 

[Aerdan]

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.

 

[Pazyryk]

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.

 

[ww2commander]

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.

I almost spat my coffee out my nose laughing after reading this!

Its funny cause its true for me too!

 

[Horem]

ART_DEF_VEFFECT_FIGHTER_MACHINE_GUN_HIT_$(TERRAIN)

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.

 

[Pazyryk]

@Horem,

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