feat: environment variables interpolation

[smoke]

closes #347

It is disabled by default to keep the existing behavior given there may
be many existing scripts using ${var} for other purposes.

Can be controlled by the following annotations:

• -- +goose ENVSUBST ON annotation enables it for all the statements following it, to the end of the file or until switched off
• -- +goose ENVSUBST OFF annotation disables it for all the statements following it, to the end of the file or until switched on

[mfridman]

There's only one oustanding issue,

CREATE OR REPLACE FUNCTION test_func()
RETURNS void AS $$
BEGIN
	RAISE NOTICE '${GOOSE_ENV_NAME}';
END;
$$ LANGUAGE plpgsql;

Returns:

CREATE OR REPLACE FUNCTION test_func()
RETURNS void AS $
BEGIN
	RAISE NOTICE 'foo';
END;
$ LANGUAGE plpgsql;

Note the escaped $$ gets escaped, which for our purposes is not good. We want to preserve $$

Added a test case in 1740b96 (which is currently failing).

[smoke]

@mfridman I believe ability to disable further substition in a file is important, there could be huge initial migrations where one could be interested in replacing environment variables only in certain small parts of it and ignore the rest.

Suggested change

	case "+goose ENVSUB ON":
	useEnvsub = true
	continue
	case "+goose ENVSUB ON":
	useEnvsub = true
	continue
	case "+goose ENVSUB OFF":
	useEnvsubst = false
	continue

[smoke]

There's only one oustanding issue,

CREATE OR REPLACE FUNCTION test_func()
RETURNS void AS $$
BEGIN
	RAISE NOTICE '${GOOSE_ENV_NAME}';
END;
$$ LANGUAGE plpgsql;

Returns:

CREATE OR REPLACE FUNCTION test_func()
RETURNS void AS $
BEGIN
	RAISE NOTICE 'foo';
END;
$ LANGUAGE plpgsql;

Note the escaped $$ gets escaped, which for our purposes is not good. We want to preserve $$

Added a test case in 1740b96 (which is currently failing).

@mfridman This is actually expected and intented way to output a literal $ sign - see https://github.com/buildkite/interpolate/blob/master/parser.go#L27
as far as I remember the drone/envsubst behaves the same https://github.com/drone/envsubst/blob/179042472c46cbed86d44ecbb11a03236efbb73a/parse/scan.go#L41C2-L41C11

So one option would be to have -- +goose ENVSUB OFF for that section, the other would be to mind the escape charracters and use $$$$ ...

[mfridman]

Hmm, you bring up good points. I slimmed the initial PR down to see what it'd look like if we started with the simplest implementation (a single annotation). I also forked buildkite/interpolate package because it was well-written and appeared to be easy to extend (if needed).

If we add the -- +goose ENVSUB OFF annotation, then it does solve the $$ problem, i.e.,

option 1 (explicit substitution)

Tip

This is likely what we'll recommend, wdyt @smoke

-- +goose StatementBegin
CREATE OR REPLACE FUNCTION test_func()
RETURNS void AS $$
BEGIN
-- +goose ENVSUB ON
	RAISE NOTICE '${GOOSE_ENV_NAME}';
-- +goose ENVSUB OFF
END;
$$ LANGUAGE plpgsql;
-- +goose StatementEnd

option2 (entire file, but disable escapes as needed)

Here you could have substitution enabled for the whole file, but disable the places where it should be avoided, such as PL/pgSQL . This isn't a great experience IMO.

-- +goose ENVSUB ON
-- +goose Up

-- +goose StatementBegin
CREATE OR REPLACE FUNCTION test_func()
-- +goose ENVSUB OFF
RETURNS void AS $$
-- +goose ENVSUB ON
BEGIN
	RAISE NOTICE '${GOOSE_ENV_NAME}';
END;
-- +goose ENVSUB OFF
$$ LANGUAGE plpgsql;
-- +goose ENVSUB ON
-- +goose StatementEnd

I presume folks will want substitution in PL/pgSQL functions. We could also error for now while we think of how to handle this case.

[mfridman]

I brought back the +goose ENVSUB OFF annotation in aac35f5

Need to sleep on this one, it does solve the problem. But it feels a bit off. Ideally, we could set a -- +goose ENVSUB ON at the top of the file and it is clever enough to ignore certain identifiers such as $$. I don't know if others will find this behaviour confusing though, but it's at odds with some SQL semantics.

We're also not aiming to support the full range of POSIX parameter expansion formats, so maybe special casing certain parts may be an option. (This was the goal of forking buildkite/interpolate package).

TL;DR - we'll probably end up going with the +goose ENVSUB OFF annotation as @smoke originally had it. I can't think of a better solution.

• Update changelog and README

[smoke]

I am a bit biased, I am using fluxcd with it embedding drone/envsubst package, so it is quite common to me that $$ is used for literal $ on those files I enable substitution.
It is a bummer the first 2 times you stumble upon it, but then you get your way around it, as long as there are options.
So I think leave the default behavior of $$ and add +goose ENVSUB OFF is the best way to go.

[mfridman]

Alright, let's just ship it.

Based on feedback we can determine if we need to swap out the interpolate package. Another good one I found was:

https://github.com/a8m/envsubst

But this will all depend on how much folks need. I think the handful of expressions we support is a good starting point.

[smoke]

@mfridman That is awesome! Still given it is quite common to use $$ in psql and others it is worth making a point right in the README.md of which library is used, as well the $$ case. Please consider that.

[mfridman]

I added a section on this in the README re. explicit wrapping. Thanks for putting the original PR together.