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

A syntax parsing error when executing SQL queries #1051

Open
key7men opened this issue Aug 16, 2021 · 2 comments
Open

A syntax parsing error when executing SQL queries #1051

key7men opened this issue Aug 16, 2021 · 2 comments

Comments

@key7men
Copy link

key7men commented Aug 16, 2021

Sqlpad version:

6.6.0

Description:

  • step 1: create a postgresql connection
  • step 2: create a table
CREATE TABLE employees (
  employee_no integer PRIMARY KEY,
  name text,
  department text
);

CREATE TABLE employee_dept_changes (
  employee_no integer NOT NULL,
  name text,
  department text,
  changed_on TIMESTAMP(6) NOT NULL
);
  • step3: insert data
INSERT INTO employees VALUES 
(1221, 'John Smith', 'Marketing'),
(1222, 'Bette Davis', 'Sales'),
(1223, 'Lucille Ball', 'Operations'),
(1224, 'John Zimmerman', 'Sales'); 
  • step4: create a function
CREATE OR REPLACE FUNCTION record_dept_changes() 
RETURNS TRIGGER AS
$$
BEGIN 
IF NEW.department <> OLD.department 
THEN INSERT INTO employee_dept_changes(employee_no, name, department, changed_on) 
VALUES(OLD.employee_no, OLD.name, OLD.department, now()); 
END IF; 
RETURN NEW; 
END;
$$
LANGUAGE 'plpgsql';

problem snapshot:

image

reason:

The service program parses the SQL query statements entered by the user in the SQL editor. When I check the code in server/model/batches.js:
image

I found the program to use sqlLimiter for determining how to parse out an independently executable SQL statement.

But, PostgreSQL uses specific syntax to embed multiple SQL statements within a single SQL statement to create stored procedures like the following (note the '$$' tag):

CREATE OR REPLACE FUNCTION dummy(IN dummy_arg varchar)
  RETURNS varchar LANGUAGE plpgsql AS $$
DECLARE
  dummy_result varchar;
BEGIN
  select concat('dummy(', dummy_arg, ')') into dummy_result;
  return dummy_result;
END $$;

Please considering optimizing the sql-limiter parser.

@rickbergfalk
Copy link
Collaborator

Thanks for this thorough repro and analysis @key7men!

Will have to think a bit on how to address this. sql-limiter uses moo to parse SQL, but unfortunately lacks a means of handling dollar quoted strings. no-context/moo#132

Will have to give this some thought. Some ideas in mind:

  • Add support for $$ for short-term, and treat everything inside as a string, so that it would be included in proper statement
  • Add some sort of pre-check step that tries to detect dollar quotes. Those specific dollar quotes can be passed in to sql-limiter, and moo parser can be generated on the fly

@key7men
Copy link
Author

key7men commented Aug 17, 2021

Thanks for this thorough repro and analysis @key7men!

Will have to think a bit on how to address this. sql-limiter uses moo to parse SQL, but unfortunately lacks a means of handling dollar quoted strings. no-context/moo#132

Will have to give this some thought. Some ideas in mind:

  • Add support for $$ for short-term, and treat everything inside as a string, so that it would be included in proper statement
  • Add some sort of pre-check step that tries to detect dollar quotes. Those specific dollar quotes can be passed in to sql-limiter, and moo parser can be generated on the fly

For now, to solve this recognition problem, I added an extra judgment as a temporary solution

  async create(batch) {
    let createdBatch;

    const queryText = batch.selectedText || batch.batchText;

    // sqlLimiter could fail at parsing the SQL text
    // If this happens the error is captured and reported as if it were a query error

    let error;
    let statementTexts = [queryText];
    try {
      statementTexts = sqlLimiter
        .getStatements(queryText)
        .map((s) => sqlLimiter.removeTerminator(s))
        .filter((s) => s && s.trim() !== '');

      if (queryText.includes('$$')) {
        statementTexts = assembleForPG(statementTexts);
      }
    } catch (e) {
      error = e;
    }

...

function assembleForPG(list) {
  let tmp = '';
  let result = [];
  const len = list.length;
  for (let i = 0; i < len; i++) {
    if (list[i].indexOf('$$') !== -1) {
      if (list[i].indexOf('$$') === list[i].lastIndexOf('$$')) {
        if (tmp) {
          tmp = tmp + list[i] + ';';
          result.push(tmp);
          tmp = '';
        } else {
          tmp = tmp + list[i] + ';';
        }
      }
    } else if (tmp) {
      tmp = tmp + list[i] + ';';
    } else {
      result.push(list[i]);
    }
  }
  return result;
}

Next I will try to optimize sql-limiter

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

No branches or pull requests

2 participants