1

i'm using something like that to define a SQL insert command:

sql := 'insert into table(a, b, c) values (' + formatfunction(a) + ', ' + 
                                               formatfunction(b) + ', ' +
                                               formatfunction(c) + ');';

after the assignment, the variable sql contains the formatted values ​​of a, b, c in a different order, resulting in something like this:

insert into (a, b, c) values ​​('value in c', 'value in a', 'value in b');

may be some problem with the compiler optimization?

sorry about the poor sample code, full executable code below:

procedure Execute(var v : String);
  function ExtractValue(var Content: String; Separator: Char = '|'): String;
  var
    vpHead,
    vpTail,
    vpContent: PChar;
    vsValor: String;
  begin
    vpContent := PChar(Content);
    Result := '';
    if (vpContent = nil) or
       (vpContent^=#0) then
      Exit;
    vpTail := vpContent;
    vpHead := vpTail;

    while not (CharInSet(vpTail^, [Separator]) or (vpTail^=#0)) do
      vpTail := StrNextChar(vpTail);

    if (vpHead^ <> #0) then
    begin
      if (vpHead <> vpTail) then
      begin
        SetString(vsValor, vpHead, vpTail - vpHead);
        Result := vsValor;
      end
      else
        Result := '';
    end;

    Content := Copy(Content, Length(vsValor) + 2, (Length(Content) - Length(vsValor)) + 1);
  end;

  function FormatAsDate(const s: String): TDate;
  begin
    Result := 0;

    if Trim(s) <> '' then
      Result := StrToDateDef(Copy(s, 1, 2) + '/' + Copy(s, 3, 2) + '/' + Copy(s, 5, 4), 0);
  end;

  function AsCurrency(const s: string): Double;
  begin
    Result := StrToFloatDef(s, 0);
  end;

  function AsDate(const s: string): string;
  var
    d: TDate;
  begin
    d := FormatAsDate(s);

    if d = 0 then
      Result := QuotedStr('null')
    else
      Result := QuotedStr(FormatDateTime('yyyy/mm/dd', d));
  end;

  function AsText(const s: string): string;
  begin
    Result := QuotedStr(s);
  end;
begin
  v :=
    'INSERT INTO TABLE (A, B, C, D, E, F, G, H, I, J, K, L, M) VALUES (' +
     AsText(ExtractValue(v)) + ', ' +
     AsText(ExtractValue(v)) + ', ' +
     AsText(ExtractValue(v)) + ', ' +
     AsText(ExtractValue(v)) + ', ' +
     AsText(ExtractValue(v)) + ', ' +
     AsText(ExtractValue(v)) + ', ' +
     AsDate(ExtractValue(v)) + ', ' +
     AsDate(ExtractValue(v)) + ', ' +
     StringReplace(FloatToStr(AsCurrency(ExtractValue(v))), ',', '.', []) + ', ' +
     StringReplace(FloatToStr(AsCurrency(ExtractValue(v))), ',', '.', []) + ', ' +
     AsText(ExtractValue(v)) + ', ' +
     StringReplace(FloatToStr(AsCurrency(ExtractValue(v))), ',', '.', []) + ', ' +
     StringReplace(FloatToStr(AsCurrency(ExtractValue(v))), ',', '.', []) + ');';
end;

using the input string '04368898000106|06|00|||3413572|26102011|31102011|1656,81|334,57||0,00|0,00', the values ​​of the insert command are in a different order than presented in the input string.

Rob Kennedy
  • 161,384
  • 21
  • 275
  • 467
tetri
  • 21
  • 4
  • 1
    @tetri: Mason is right. You have some bug somewhere else in your code. – Andreas Rejbrand Jun 15 '12 at 18:08
  • I'm sure like a lot of offices around the world we use this exact string concatenation in our SQL queries and have never experienced a similar issue. My experience is limited to 2010 and XE2 but looking at your sample code I can't see a reason why you would get this behavior. More detailed code would be helpful. – James West Jun 15 '12 at 18:20
  • if this line of code that you posted is exactly the same line from your code, set a breakpoint and interrogate the values before they are sent to through the format function... I agree with the rest of the comments, chances are your bug is elsewhere. – GDF Jun 15 '12 at 18:28
  • If I understand correctly, you're missing the single-quote escape, i.e. sql := 'insert into table(a, b, c) values ( '' ' + formatfunction(a) + ' '', '' ' + formatfunction(b) + ' '', '' ' + formatfunction(c) + ''' );'; copy paste this code and see if it works. –  Jun 15 '12 at 19:26
  • @Dorin If the real code was written with `=` instead of `:=`, then it would be a compile error. – David Heffernan Jun 15 '12 at 19:32
  • 4
    Also, you should never ever build a SQL query that way. Concatenation leaves you open to SQL Injection attacks, a serious security hole that someone could use to hack into your system. Please don't write another line of SQL-generating code until you've read up on SQL injection and how to prevent it. (Not exaggerating. I mean it. Stop right now, before you cause a security breach.) – Mason Wheeler Jun 15 '12 at 19:33
  • +1 The update to the question is good and now it all makes sense. – David Heffernan Jun 15 '12 at 20:13
  • What a coincidence that this comes up so soon after this closely related question: http://stackoverflow.com/questions/11010456/in-delphi-are-parameters-evaluated-in-order-when-passed-into-a-method – David Heffernan Jun 15 '12 at 20:14

1 Answers1

9

Your code depends on the evaluation order of operands in an expression. That evaluation order is not defined.

Looking at a simpler case, consider this code:

x := f1(a) + f2(b);

There is no guarantee over which order the function calls f1() and f2() will be executed. You, presumably, expect f1() to execute before f2() but the compiler does not guarantee that and in practice I believe that these operands will usually be evaluated right to left.

In your code, the operands in your expression involve a function call, ExtractValue(), that has a side effect which modifies its argument. Since the operands in your expression are not evaluated left to right, the calls to ExtractValue() do not occur in the same order as they appear in the expression. And since ExtractValue() has a side-effect which influences the rest of the expression, the results are dependent on the evaluation order.

You will need to re-work this code so that the calls to ExtractValue() occur in separate statements.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490