Proposal: Format SQL in PL/pgSQL EXECUTE and BigQuery EXECUTE IMMEDIATE

[arnodirlam]

Would you be open to formatting literal SQL commands inside PL/pgSQL EXECUTE and BigQuery EXECUTE IMMEDIATE? This is a proposal for #81.

For example, with the PostgreSQL parser and default formatting options:

DO $$BEGIN EXECUTE $sql$update widgets set enabled=true where id=$1$sql$ USING 1; END$$;

becomes:

DO $$
BEGIN
  EXECUTE
    $sql$
      UPDATE widgets SET enabled = TRUE WHERE id = $1;
    $sql$
  USING 1;
END;
$$;

With the BigQuery parser:

EXECUTE IMMEDIATE "select @id" INTO result USING 1 AS id;

becomes:

EXECUTE IMMEDIATE
  r'''
    SELECT @id;
  '''
INTO result
USING 1 AS id;

PostgreSQL commands retain their dollar-quote delimiters. BigQuery commands use raw triple quotes to preserve backslashes, choosing single or double quotes when possible. Computed commands, unparseable SQL, and BigQuery commands containing both triple-quote delimiters stay unchanged.

Further embedded languages inside commands stay untouched to avoid introducing conflicting delimiters. As discussed, this safeguard stays in this PR. I'll address quote-style preservation and revisit nested formatting in a separate PR.

I'd welcome your thoughts on the layout. Feel free to close this PR if it isn't wanted.

[nene]

Thanks for the PR.

Wrote some questions.

[nene]

I don't quite follow. How will nested embedding introduce new dollar delimiters? Perhaps you could provide an example.

[arnodirlam]

I ran into this case:

DO $$BEGIN
  EXECUTE $sql$
    CREATE FUNCTION f() RETURNS text
    LANGUAGE sql AS 'select ''value'''
  $sql$;
END$$;

Formatting the inner function body changes its quotes to $$, closing the outer DO block too early.

We could leave the inner body untouched, as this PR currently proposes. Or we could change conflicting delimiters, for example using $pgfmt1$ around the outer block, so nested formatting remains possible.

I tried the second approach locally. It takes about 25 added lines and works for this example and deeper nesting. It currently renders each embedded body an extra time to check for conflicts, so that part needs more scrutiny.

Which would you prefer? Given the small PoC, I now lean toward exploring safe delimiter selection.

[nene]

Aha... I see now.

The problem stems from the fact that the formatter changes quotes from '...' style to $$..$$ style.

I'm thinking that it might be better to actually drop this quote-style change feature completely. It seems to cause more trouble than there's to be gained from it. Frankly I've never really felt a personal need for this, it just felt like something that would be neat to have. But in practice I always write such SQL bodies using dollar-quoted strings to begin with.

So, my proposal would be: when the function body is quoted by anything other then dollar-quoted string, we'd just leave it as is and don't format SQL inside it. Only when it's dollar-quoted will we format its contents.

[nene]

This also raises the question of what to do with BigQuery. I think we should take the same route there - just leave it unformatted unless it's already inside r''' ''' or r""" """ quotes. Similarly to Postgres there are several of different ways of quoting strings, but there's pretty much just one style which the BigQuery docs promote for use in function definitions.

[nene]

However, this whole behavioral change is probably a bit too much to squeeze into this pull request.

I think better to tackle this separately. Either you can tackle it by yourself after this PR, or I'll take it over. I'm OK either way.

Until then, we can live with this embeddedLanguageFormatting-off workaround.

[arnodirlam]

That makes sense to me. I’ll keep the workaround here and take on the quote-style change in a separate PR after this one merges. I’ll also check whether that lets us remove the nesting workaround safely.

[nene]

Created an issue to avoid forgetting about this: #87

[nene]

Not for myself: I really should rename the execute_expr to execute_immediate_expr. Currently it looks like execute_expr is the expression-counter-part for exectute_stmt, but these are quite different things.

[nene]

Created issue in parser: nene/sql-parser-cst#137

[nene]

One more comment.

[nene]

Thanks! That's looking good now.