Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When command contains comment and there is a "@something" you got error that you need to declare parameter [DNET176] #184

Open
firebird-automations opened this issue Sep 14, 2008 · 10 comments

Comments

@firebird-automations
Copy link

Submitted by: @cincuranet

Relate to DNET564
Is related to DNET761

Attachments:
FbCommand.patch
FbCommand.patch

Votes: 2

When command contains comment and there is a "@something" you got error that you need to declare parameter.

I.e.:
select foo from bar --abc @xyz

@firebird-automations
Copy link
Author

Commented by: Gareth Goslett (gareth)

A patch to fix this issue, not extensively tested.

The comment text is removed from the statement but the comment characters '--' are not.
Would it be more efficient to parse the complete statement, removing whitespace, carriage returns and comments?

One reason to keep comments in is that you can now view the statement from the monitoring tables.

I ran the following "sql" text and it executed fine.

"INSERT --An insert statement\r\n" +
" INTO -- Define the table next\r\n" +
" TESTING_PK\r\r\n--The table\r\n" +
" ( -- Separator @data1 \r\n" +
" FIELD_01, -- First field, links to @FIELD_01\r\n" +
" FIELD_02 -- Second field, links to @FIELD_02\r\n" +
" ) -- Separator @data2 \r\n" +
" VALUES" +
" ( -- Separator @data3 \r\n" +
" COALESCE((SELECT MAX(FIELD_01) FROM TESTING_PK) + @FIELD_01, 1), --@FIELD_01 - Get the next --@max of field.\r\n" +
" '--' || @FIELD_02 || '--'--@add text parameter\r\n" +
" ) -- Separator @data4 \r\n" +
" RETURNING FIELD_01-- Comment @td"

@firebird-automations
Copy link
Author

Modified by: Gareth Goslett (gareth)

Attachment: FbCommand.patch [ 11373 ]

@firebird-automations
Copy link
Author

Commented by: @cincuranet

First, the patch is wrong. Try query like "select 'foo' from rdb$database /* @foo */". Secondly, I don't wanna to go this way. We need fast, simple and reliable way to detect and skip (the comment can be pretty long) comments in query, not to extending the method with thousands of ifs.

@firebird-automations
Copy link
Author

Commented by: Gareth Goslett (gareth)

Can we strip comments and control characters entirely, only send the query text to the server ?
If so, we can first strip all comments and others, then parse for parameters.

I was not aware that firebird handled the c style comments, I thaught it only allowed the '--' comment characters, thanks.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

We can, if somebody creates some reliable function for it. But for now, I don't see this as a big issue.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

This is final version from the mailing list thread [http://www.nabble.com/FbCommand-sql-parser-td22215029.html] from Gareth Goslett.

Looks good, I'll just create couple of coding style tweaks.

string ParseNamedParameters(string sql)
{
// Clear old parameters.
this.namedParameters.Clear();

      // We do not need to parse if there are no parameters\.
      if \(sql\.IndexOf\('@'\) == \-1\)
      \{
        return sql;
      \}

      int           index;
      int           end           = sql\.Length \- 1;
      StringBuilder resultBuilder = new StringBuilder\(sql\.Length\);

      for \(int i = 0, j = sql\.Length; i < j; i\+\+\)
      \{
        switch \(sql\[i\]\)
        \{
          // Read a named parameter\.
          case '@':
            for \(index = i \+ 1; index < j &&

(Char.IsLetterOrDigit(sql[index]) || sql[index] == '_' || sql[index]
== '$'); index++);
resultBuilder.Append('?');
this.namedParameters.Add(sql.Substring(i, index - i));
i = index - 1;
continue;

          // Read quoted text\.
          case '\\'':
          case '"':
            for \(index = i \+ 1; index < j && sql\[index\] \!= sql\[i\]; index\+\+\);
            if\(index \>= j\)
            \{
              resultBuilder\.Append\(sql\.Substring\(i\)\);
            \}
            else
            \{
              resultBuilder\.Append\(sql\.Substring\(i, index \+ 1 \- i\)\);
            \}
            i = index;
            continue;

          // Read a single line comment\.
          case '\-':
            if \(\(i < end\) && \(sql\[i \+ 1\] == '\-'\)\)
            \{
              for \(index = i \+ 1; index < j && \(sql\[index\] \!= '\\r'

&& sql[index] != '\n'); index++);
resultBuilder.Append(sql.Substring(i, index - i));
i = index - 1;
continue;
}

            resultBuilder\.Append\('\-'\);
            continue;

          // Read a block comment\.
          case '/':
            if \(\(i < end\) && \(sql\[i \+ 1\] == '\*'\)\)
            \{
              while \(i < j\)
              \{
                resultBuilder\.Append\(sql\[i\]\);
                if \(sql\[i\] == '\*' && i < end && sql\[i \+ 1\] == '/'\)
                \{
                  break;
                \}

                i\+\+;
              \}

              continue;
            \}

            resultBuilder\.Append\('/'\);
            continue;

          // Read content\.
          default:
            resultBuilder\.Append\(sql\[i\]\);
            continue;
        \}
      \}

      return resultBuilder\.ToString\(\);
    \}

@firebird-automations
Copy link
Author

Commented by: Gareth Goslett (gareth)

Patch for svn, 908.

@firebird-automations
Copy link
Author

Modified by: Gareth Goslett (gareth)

Attachment: FbCommand.patch [ 11440 ]

@firebird-automations
Copy link
Author

Modified by: @cincuranet

Link: This issue relate to DNET564 [ DNET564 ]

@firebird-automations
Copy link
Author

Modified by: @cincuranet

Link: This issue is related to DNET761 [ DNET761 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants