Bad indentation for DECLARE in PostgreSQL

The DECLARE statement in PL/pgSQL is similar to the one in PL/SQL and is used to declare variables: https://www.postgresql.org/docs/current/plpgsql-declarations.html

Test case in PostgreSQL:

-- Current behaviour
CREATE FUNCTION less_than(a text, b text) RETURNS boolean AS $$
  DECLARE
  local_a text := a;
  local_b text := b;
  BEGIN
    RETURN local_a < local_b;
  END;
$$ LANGUAGE plpgsql;

-- Expected behaviour
CREATE FUNCTION less_than(a text, b text) RETURNS boolean AS $$
  DECLARE
    local_a text := a;
    local_b text := b;
  BEGIN
    RETURN local_a < local_b;
  END;
$$ LANGUAGE plpgsql;

However, there is also a DECLARE statement that creates a cursor: https://www.postgresql.org/docs/12/sql-declare.html

-- Current behaviour
DECLARE
liahona
  CURSOR FOR
	   SELECT *
	   FROM films;

-- Expected
DECLARE
  liahona
  CURSOR FOR
    SELECT *
      FROM films;

I think sqlind-beginning-of-block probably needs to handle the PostgreSQL case:

emacs-sql-indent/sql-indent.el

Lines 956 to 957 in 56be397

    
           (when (eq sql-product 'oracle) ; declare statements only start blocks in PL/SQL 
        
             (sqlind-maybe-declare-statement))

However, for the cursor use-case, I think that sqlind-maybe-declare-statement needs to understand if it’s in-begin-block and do something special, especially in order to recognize and indent the embedded SELECT or VALUES statement.

Thanks for reporting this. I pushed a fix, but unfortunately this had to change how existing indentations work, hopefully for the better. PRs #88, #89 and #67 are affected by this change.

Can you please check to see if it is OK for your code?

First, thank you for working on this so quickly.

However, it looks like there are two corner cases that aren’t covered.

The first is when a DECLARE follows another DECLARE, which I think can be detected by just checking for declare-statement:

-- Current behaviour
CREATE FUNCTION less_than(a text, b text) RETURNS boolean AS $$
  DECLARE local_a text := a;
    DECLARE local_b text := b;
  BEGIN
    RETURN local_a < local_b;
  END;
$$ LANGUAGE plpgsql;

-- Expected behaviour
CREATE FUNCTION less_than(a text, b text) RETURNS boolean AS $$
  DECLARE local_a text := a;
  DECLARE local_b text := b;
  BEGIN
    RETURN local_a < local_b;
  END;
$$ LANGUAGE plpgsql;

The second happens with nesting inside another BEGIN:

-- Current behaviour
CREATE FUNCTION less_than(a text, b text) RETURNS boolean AS $$
  BEGIN
    DECLARE
      local_a text := a;
    local_b text := b;
    BEGIN
      RETURN local_a < local_b;
    END;
  END;
$$ LANGUAGE plpgsql;

-- Expected behaviour
CREATE FUNCTION less_than(a text, b text) RETURNS boolean AS $$
  BEGIN
    DECLARE
      local_a text := a;
      local_b text := b;
    BEGIN
      RETURN local_a < local_b;
    END;
  END;
$$ LANGUAGE plpgsql;

This might require a new sqlind-in-function defun to handle all the cases where another DECLARE can happen (i.e. as the first block, within another block, inside a conditional, etc.):

-- Current behaviour
CREATE FUNCTION less_than(a text, b text) RETURNS boolean AS $$
  BEGIN
    IF a IS NULL THEN
      RETURN FALSE;
    ELSE
      DECLARE
	local_a text := a;
      local_b text := b;
      BEGIN
	RETURN local_a < local_b;
      END;
    END IF;
  END;
$$ LANGUAGE plpgsql;

-- Expected behaviour
CREATE FUNCTION less_than(a text, b text) RETURNS boolean AS $$
  BEGIN
    IF a IS NULL or b IS NULL THEN
      RETURN FALSE;
    ELSE
      DECLARE
	local_a text := a;
	local_b text := b;
      BEGIN
	RETURN local_a < local_b;
      END;
    END IF;
  END;
$$ LANGUAGE plpgsql;

Thanks for preparing the test cases, I don't use Postgresql and I rely on these to improve sql-indent. I have pushed a fix for the test cases you prepared. Can you please have a look? Thanks, Alex.

I appreciate the effort to support Postgres!

Sadly, I have found a corner case that results from only examining the previous token: e7795c7#r37366259

Sadly, I have found a corner case that results from only examining the previous token

Unlike an SQL parser, sql-indent cannot afford to parse the entire file from the start every time the user wants to indent a line, this would be too slow. Instead, sql-indent relies on searching backwards for a good starting point plus a collection of heuristics to determine the context of the line, so it can be indented. This mechanism means that you can easily find many corner cases which don't work correctly. I am trying to handle all reasonable cases, but I have limited time to work on this package and for the time being, I will just document this in the Limitations section of sql-indent.org.

sqlind-maybe-declare-statement is the sqlind-in-declare function you suggested, but I am not sure how it should be implemented: I need to come up with a mechanism to determine if the "declare" keyword starts a block with multiple declarations, or it is just a single statement.

I just want to be clear that I think your report is valid, but unfortunately I don't know how to fix it and have no more time to look into it.

	(when (eq sql-product 'oracle) ; declare statements only start blocks in PL/SQL
	(sqlind-maybe-declare-statement))